Menu
▾
▴
[sdcc-devel] [ sdcc-Bugs-3441816 ] Old ralloc passes wrong value to callee
|
From: SourceForge.net <no...@so...> - 2011-12-05 08:54:49
|
Bugs item #3441816, was opened at 2011-11-24 07:07 Message generated for change (Comment added) made by spth You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=3441816&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: z80 port Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Brian Ruthven (u6c87) Assigned to: Nobody/Anonymous (nobody) Summary: Old ralloc passes wrong value to callee Initial Comment: During development, I use --ralloc to compile quickly. In one file, I have a construct similar to this: if ((ptr = alloc_wrapper()) == NULL) return; /* other stuff here */ err = func(foo, ptr); However on running the compiled code, I found NULL was passed to func() instead of ptr. Looking at the assembler (which is quite complex), it looks like the address of ptr stored on the stack is miscalculated. It just so happens that a NULL is located which is why it stood out. If I compile the same code (unmodified) without --oldralloc I get the correct results. Originally found using 20111022-6988, I have reproduced this using today's src snapshot: SDCC : z80 3.1.0 #7066 (Nov 24 2011) (Solaris i386) The function and file are quite complex, so it will take me a little while to strip this down to a more self-contained testcase. If this is likely to be looked into and fixed, then I'll spend the time doing this and attach a testcase. [ PS. Sorry if this is a duplicate - I tried logging it earlier today, but I can find no evidence it was actually accepted ] ---------------------------------------------------------------------- >Comment By: Philipp Klaus Krause (spth) Date: 2011-12-05 00:54 Message: This is really a bug in the old register allocator. The new register allocator has a mechanism to avoid this problem in add_operand_conflicts_in_node() in ralloc2.cc. Philipp ---------------------------------------------------------------------- Comment By: Brian Ruthven (u6c87) Date: 2011-12-02 02:40 Message: I've added a strippped down version of the offending function ("Testcase attempt #1"). It's gibberish from a programming point of view, but if you compile using "sdcc -mz80 --oldralloc --no-peep --fverbose-asm -c", you see the offending section of assembler leading up to the second kread() call at line 283: 00DD 79 281 ld a,c 00DE C6 04 282 add a, #0x04 00E0 5F 283 ld e,a <---- 'e' is overwritten here 00E1 7B 284 ld a,e <---- and is re-loaded here 00E2 CE 00 285 adc a, #0x00 00E4 47 286 ld b,a There are two sections bracketed by "#if 1/#endif". If either of these are changed to "#if 0", the resulting register allocation is disturbed, and it makes it possible to use a register pair (probably de) rather than being split across arbitrary pairs such as bd and ce. I've included all the struct declarations in case the layout of one of these is triggering this behaviour. As noted, the program is gibberish and would never run in its current form, but inspection of the resulting assembler should be possible. ---------------------------------------------------------------------- Comment By: Philipp Klaus Krause (spth) Date: 2011-11-29 01:37 Message: Can you provide a testcase? While we prefer small examples, a single function in a single file is better than nothing, even if it is a big function. Philipp ---------------------------------------------------------------------- Comment By: Brian Ruthven (u6c87) Date: 2011-11-25 04:20 Message: As best I can determine, this bug is *not* present in sdcc 3.0.0. The failing section looks like this ([ec] holds the bas address of hdr): 1813 ; genPlus 1814 ; genPlusIncr 0767 79 1815 ld a,c 0768 C6 04 1816 add a,#0x04 1817 ; Addition result is in same register as operand of next addition. 076A C5 1818 push bc 076B 4F 1819 ld c, a <---- (*) 076C 7B 1820 ld a, e 076D 47 1821 ld b, a 076E 79 1822 ld a, c 076F 5F 1823 ld e, a 0770 78 1824 ld a, b 0771 C1 1825 pop bc 0772 CE 00 1826 adc a,#0x00 At line 1819, it appears the register allocator chose 'c' as the destination. This is the high byte of &hdr, but is preserved with the push. By the time this section of code is finished, 'e' is trashed, but is not used again, so that's OK. My simpler testcases all end up using proper register pairs for the addresses, so I guess this could be a result of using ad hoc "pairs" such as ec and bd rather than bc, hl, etc... ---------------------------------------------------------------------- Comment By: Brian Ruthven (u6c87) Date: 2011-11-25 03:01 Message: I'll do my best, but I've needed some bug fixes along the way which caused me massive problems (3188357 being the worst for me which caused a complete wipe of memory). I've tried with --no-peep and sdcc #6988. I think I've tracked it down to this section of assembler when constructing the args for a call (sorry it's not more stripped down). Some context: Call is: err = kread(fd, (void *)hdr.eh_load_addr, hdr.eh_file_len, &rbytes); hdr is declared as below: typedef struct exec_hdr_s { char eh_magic[4]; uint16_t eh_load_addr; /* 0 = relocatable */ uint16_t eh_file_len; /* Bytes to read from disk */ uint16_t eh_exec_size; /* Bytes to allocate */ uint16_t eh_reloc_len; /* Length of the reloc table */ } exec_hdr_t; Here's the verbose assembler leading up to the call (I've omitted the calculation of &rbytes as this is correct): 1974 ; genAddrOf 073D 21 13 00 1975 ld hl,#0x0013 0740 39 1976 add hl,sp 0741 4D 1977 ld c,l 0742 5C 1978 ld e,h <--- address of hdr in [ec] 1979 ; genPlus 1980 ; genPlusIncr 0743 79 1981 ld a,c 0744 C6 06 1982 add a, #0x06 0746 57 1983 ld d,a 0747 7B 1984 ld a,e 0748 CE 00 1985 adc a, #0x00 074A 47 1986 ld b,a <--- [bd] contains &len 1987 ; genPointerGet 1988 ; AOP_STK for _proc_create_sloc2_1_0 1989 ;fetchPairLong 074B 6A 1990 ld l,d 074C 60 1991 ld h,b 074D 7E 1992 ld a,(hl) 074E DD 77 DD 1993 ld -35 (ix),a 0751 23 1994 inc hl 0752 7E 1995 ld a,(hl) 0753 DD 77 DE 1996 ld -34 (ix),a <--- -34/-35(ix) contains len 1997 ; genPlus 1998 ; genPlusIncr 0756 79 1999 ld a,c 0757 C6 04 2000 add a, #0x04 0759 5F 2001 ld e,a <-----------------****** 'e' is overwritten ****** 075A 7B 2002 ld a,e 075B CE 00 2003 adc a, #0x00 075D 47 2004 ld b,a <--- [be] contains &addr 2005 ; genPointerGet 2006 ;fetchPairLong 075E 6B 2007 ld l,e 075F 60 2008 ld h,b 0760 7E 2009 ld a,(hl) 0761 23 2010 inc hl 0762 66 2011 ld h,(hl) 0763 6F 2012 ld l,a <--- [hl] should contain addr 2013 ; genCast 2014 ;fetchPairLong 0764 5D 2015 ld e,l 0765 54 2016 ld d,h <--- save in [de] 2017 ; genIpush 2018 ; _saveRegsForCall: sendSetSize: 0 deInUse: 0 bcInUse: 0 deSending: 0 2019 ; AOP_STK for _proc_create_sloc0_1_0 2020 ;fetchPairLong 0766 DD 6E DF 2021 ld l,-33 (ix) 0769 DD 66 E0 2022 ld h,-32 (ix) 076C E5 2023 push hl <=== arg: &rbytes 2024 ; genIpush 2025 ; AOP_STK for _proc_create_sloc2_1_0 2026 ;fetchPairLong 076D DD 6E DD 2027 ld l,-35 (ix) 0770 DD 66 DE 2028 ld h,-34 (ix) 0773 E5 2029 push hl <=== arg: len 2030 ; genIpush 0774 D5 2031 push de <=== arg: addr 2032 ; genIpush I believe the fatal part is line 2001 (offset 0x759). Here we store the partial result from the addition in register 'e', but 'e' is still holding the base address of the hdr, and we load this in the next instruction to perform the addition on the high byte. The preceeding code for &len (lines 1981-86) do this correctly. It would seem that the register allocator forgot that 'e' was still required and used it as a target register. The resulting pointer dereference reads from a random location in memory and feeds this as 'addr' to the kread() function.causing code to be loaded to the wrong address, corrupting whatever was it its way. ---------------------------------------------------------------------- Comment By: Maarten Brock (maartenbrock) Date: 2011-11-25 01:05 Message: Brian, Can you please verify if this bug was also present in SDCC 3.0.0? Maarten ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=3441816&group_id=599 |