| Message ID | 18ceeedd-2791-0c78-ba54-32bb67e5c003@gjlay.de |
|---|---|
| State | New |
| Headers | show |
Georg-Johann Lay <avr@gjlay.de> writes: > One test case used unsigned long for the 3rd parameter of memset, which > should be size_t. This made the test crash for targets where correct > parameter passing depends on correct prototypes. > > Fixed and committed as obvious. Catching up on backlog, but... I'm not sure this counts as obvious if the crash was an ICE. We should at least fail gracefully. Thanks, Richard > > Johann > > > gcc/testsuite/ > * gcc.c-torture/execute/pr30778.c (memset): Use size_t for 3rd > parameter in declaration. > > Index: gcc.c-torture/execute/pr30778.c > =================================================================== > --- gcc.c-torture/execute/pr30778.c (revision 242541) > +++ gcc.c-torture/execute/pr30778.c (working copy) > @@ -1,4 +1,4 @@ > -extern void *memset (void *, int, unsigned long); > +extern void *memset (void *, int, __SIZE_TYPE__); > extern void abort (void); > > struct reg_stat {
On 22.12.2016 19:20, Richard Sandiford wrote: > Georg-Johann Lay <avr@gjlay.de> writes: >> One test case used unsigned long for the 3rd parameter of memset, which >> should be size_t. This made the test crash for targets where correct >> parameter passing depends on correct prototypes. >> >> Fixed and committed as obvious. > > Catching up on backlog, but... I'm not sure this counts as obvious > if the crash was an ICE. We should at least fail gracefully. The crash I mentioned happened on avr where parameter passing needs correct prototypes. The code crashed when executed on avr because a long gets passed in regs R22..R25 (LSBs first) but a size_t is passed in R24..R25. Hence the implementation of memset would pick up the upper two bytes of the length (zero) which crashes when run. PR30778 was about wrong-code because more than the specified number of bytes were written. It was reported for x64_64 where we have size_t = unsigned long, hence the change is a no-op on the platform the PR was originally reported against (similar for x86 where size_t = unsigned). Moreover, the test case is condensed from GCC's combine.c and I'd expect that GCC's header are using correct prototypes :-) Johann > > Thanks, > Richard > >> >> Johann >> >> >> gcc/testsuite/ >> * gcc.c-torture/execute/pr30778.c (memset): Use size_t for 3rd >> parameter in declaration. >> >> Index: gcc.c-torture/execute/pr30778.c >> =================================================================== >> --- gcc.c-torture/execute/pr30778.c (revision 242541) >> +++ gcc.c-torture/execute/pr30778.c (working copy) >> @@ -1,4 +1,4 @@ >> -extern void *memset (void *, int, unsigned long); >> +extern void *memset (void *, int, __SIZE_TYPE__); >> extern void abort (void); >> >> struct reg_stat { >
Georg-Johann Lay <avr@gjlay.de> writes: > On 22.12.2016 19:20, Richard Sandiford wrote: >> Georg-Johann Lay <avr@gjlay.de> writes: >>> One test case used unsigned long for the 3rd parameter of memset, which >>> should be size_t. This made the test crash for targets where correct >>> parameter passing depends on correct prototypes. >>> >>> Fixed and committed as obvious. >> >> Catching up on backlog, but... I'm not sure this counts as obvious >> if the crash was an ICE. We should at least fail gracefully. > > The crash I mentioned happened on avr where parameter passing needs > correct prototypes. The code crashed when executed on avr because > a long gets passed in regs R22..R25 (LSBs first) but a size_t is > passed in R24..R25. Hence the implementation of memset would pick > up the upper two bytes of the length (zero) which crashes when run. Ah, OK, so it was the test executable that crashed and not the compiler. In that case sorry for the noise. Richard > PR30778 was about wrong-code because more than the specified number > of bytes were written. It was reported for x64_64 where we have > size_t = unsigned long, hence the change is a no-op on the platform > the PR was originally reported against (similar for x86 where > size_t = unsigned). > > Moreover, the test case is condensed from GCC's combine.c and I'd > expect that GCC's header are using correct prototypes :-) > > Johann > >> >> Thanks, >> Richard >> >>> >>> Johann >>> >>> >>> gcc/testsuite/ >>> * gcc.c-torture/execute/pr30778.c (memset): Use size_t for 3rd >>> parameter in declaration. >>> >>> Index: gcc.c-torture/execute/pr30778.c >>> =================================================================== >>> --- gcc.c-torture/execute/pr30778.c (revision 242541) >>> +++ gcc.c-torture/execute/pr30778.c (working copy) >>> @@ -1,4 +1,4 @@ >>> -extern void *memset (void *, int, unsigned long); >>> +extern void *memset (void *, int, __SIZE_TYPE__); >>> extern void abort (void); >>> >>> struct reg_stat { >>
Index: gcc.c-torture/execute/pr30778.c =================================================================== --- gcc.c-torture/execute/pr30778.c (revision 242541) +++ gcc.c-torture/execute/pr30778.c (working copy) @@ -1,4 +1,4 @@ -extern void *memset (void *, int, unsigned long); +extern void *memset (void *, int, __SIZE_TYPE__); extern void abort (void); struct reg_stat {