[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 Richard Guenther rguenth at gcc dot gnu.org changed: What|Removed |Added Priority|P3 |P2
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #36 from Eric Botcazou ebotcazou at gcc dot gnu.org 2012-02-07 17:21:44 UTC --- Author: ebotcazou Date: Tue Feb 7 17:21:36 2012 New Revision: 183974 URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=183974 Log: PR middle-end/51994 * expr.c (get_inner_reference): If there is an offset, add a negative bit position to it (if any). Added: trunk/gcc/testsuite/gcc.c-torture/execute/20120207-1.c Modified: trunk/gcc/ChangeLog trunk/gcc/expr.c trunk/gcc/testsuite/ChangeLog
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #37 from Eric Botcazou ebotcazou at gcc dot gnu.org 2012-02-07 17:24:32 UTC --- Author: ebotcazou Date: Tue Feb 7 17:24:27 2012 New Revision: 183975 URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=183975 Log: PR middle-end/51994 * expr.c (get_inner_reference): If there is an offset, add a negative bit position to it (if any). Added: branches/gcc-4_6-branch/gcc/testsuite/gcc.c-torture/execute/20120207-1.c - copied unchanged from r183974, trunk/gcc/testsuite/gcc.c-torture/execute/20120207-1.c Modified: branches/gcc-4_6-branch/gcc/ChangeLog branches/gcc-4_6-branch/gcc/expr.c branches/gcc-4_6-branch/gcc/testsuite/ChangeLog
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 Eric Botcazou ebotcazou at gcc dot gnu.org changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution||FIXED --- Comment #38 from Eric Botcazou ebotcazou at gcc dot gnu.org 2012-02-07 17:28:03 UTC --- Patch applied.
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #35 from Eric Botcazou ebotcazou at gcc dot gnu.org 2012-02-06 12:29:06 UTC --- So your patch is probably ok (can you try verifying we don't get (too much) codegen differences on a bootstrap?) There are no differences for a compiler build on Alpha and i586 (4.6 branch).
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #34 from rguenther at suse dot de rguenther at suse dot de 2012-02-02 08:56:04 UTC --- On Wed, 1 Feb 2012, ebotcazou at gcc dot gnu.org wrote: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #32 from Eric Botcazou ebotcazou at gcc dot gnu.org 2012-02-01 16:34:30 UTC --- The base object can be an indirect reference, so yes, there doesn't have to be an overall positive offset (well, yes, to the _real_ object, but we don't see that). If this is an indirect reference, there is no base object by definition. So I'm not sure we should care in get_inner_reference and, in any case, I'm not sure what to do. Probably avoid sending MEM_REF to get_inner_reference in this case, after all it's clearly not a handled_component_p-like thing. Well, you can have component refs wrapped around a MEM_REF (or formerly an INDIRECT_REF). The only difference now is that the MEM_REF may have a (negative) constant offset embedded. Now, only if the MEM_REF is based on an ADDR_EXPR (and thus a real object) we factor in its (possibly negative) offset to bitpos. So, hum - now I don't see as easily that we can get a negative bitpos from a not undefined input ... (maybe except for the Ada fat pointer case). So your patch is probably ok (can you try verifying we don't get (too much) codegen differences on a bootstrap?) Richard.
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #25 from Eric Botcazou ebotcazou at gcc dot gnu.org 2012-02-01 15:34:37 UTC --- I've changed my mind: given that we shouldn't have references outside the base object in valid programs, there must be an offset if the bitpos is negative, so we can simply add the (rounded) latter to the former. That's what the callers would have to do anyway, so we'd better factor it in get_inner_reference.
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #26 from Eric Botcazou ebotcazou at gcc dot gnu.org 2012-02-01 15:36:06 UTC --- Created attachment 26545 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=26545 Tentative fix Uros, does it fix the original issue?
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #28 from Richard Guenther rguenth at gcc dot gnu.org 2012-02-01 15:41:52 UTC --- (In reply to comment #27) (In reply to comment #25) I've changed my mind: given that we shouldn't have references outside the base object in valid programs, there must be an offset if the bitpos is negative, so we can simply add the (rounded) latter to the former. That's what the callers would have to do anyway, so we'd better factor it in get_inner_reference. I'm not sure it's the best thing to do (adjusting 'offset' by a BITS_PER_UNIT multiple instead of whatever the caller would like the most). Only the callers would know how they want to adjust for negative bitpos. Oh, and if you do that, please change bitpos to unsigned HOST_WIDE_INT *.
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #27 from Richard Guenther rguenth at gcc dot gnu.org 2012-02-01 15:41:15 UTC --- (In reply to comment #25) I've changed my mind: given that we shouldn't have references outside the base object in valid programs, there must be an offset if the bitpos is negative, so we can simply add the (rounded) latter to the former. That's what the callers would have to do anyway, so we'd better factor it in get_inner_reference. I'm not sure it's the best thing to do (adjusting 'offset' by a BITS_PER_UNIT multiple instead of whatever the caller would like the most). Only the callers would know how they want to adjust for negative bitpos.
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 Eric Botcazou ebotcazou at gcc dot gnu.org changed: What|Removed |Added Attachment #26545|0 |1 is obsolete|| --- Comment #29 from Eric Botcazou ebotcazou at gcc dot gnu.org 2012-02-01 15:49:14 UTC --- Created attachment 26547 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=26547 Correct fix This adds the missing division...
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #30 from Eric Botcazou ebotcazou at gcc dot gnu.org 2012-02-01 15:53:25 UTC --- I'm not sure it's the best thing to do (adjusting 'offset' by a BITS_PER_UNIT multiple instead of whatever the caller would like the most). Only the callers would know how they want to adjust for negative bitpos. I don't think the callers are prepared for a reference outside the base object, so there must be a offset. And I don't see what they would want to do with a negative bitpos, apart from translating it somehow or other.
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #31 from rguenther at suse dot de rguenther at suse dot de 2012-02-01 16:00:41 UTC --- On Wed, 1 Feb 2012, ebotcazou at gcc dot gnu.org wrote: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #30 from Eric Botcazou ebotcazou at gcc dot gnu.org 2012-02-01 15:53:25 UTC --- I'm not sure it's the best thing to do (adjusting 'offset' by a BITS_PER_UNIT multiple instead of whatever the caller would like the most). Only the callers would know how they want to adjust for negative bitpos. I don't think the callers are prepared for a reference outside the base object, so there must be a offset. And I don't see what they would want to do with a negative bitpos, apart from translating it somehow or other. The base object can be an indirect reference, so yes, there doesn't have to be an overall positive offset (well, yes, to the _real_ object, but we don't see that). Yes, you have to adjust for a negative _bit_ position. But fact is the whole constant offset (even if negative) is usually returned via bitpos.
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #32 from Eric Botcazou ebotcazou at gcc dot gnu.org 2012-02-01 16:34:30 UTC --- The base object can be an indirect reference, so yes, there doesn't have to be an overall positive offset (well, yes, to the _real_ object, but we don't see that). If this is an indirect reference, there is no base object by definition. So I'm not sure we should care in get_inner_reference and, in any case, I'm not sure what to do. Probably avoid sending MEM_REF to get_inner_reference in this case, after all it's clearly not a handled_component_p-like thing.
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #33 from Uros Bizjak ubizjak at gmail dot com 2012-02-01 18:41:59 UTC --- (In reply to comment #29) Created attachment 26547 [details] Correct fix This adds the missing division... This patch fixes both the testcase and original issue. Thanks!
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #17 from Jakub Jelinek jakub at gcc dot gnu.org 2012-01-26 08:02:50 UTC --- Doing it in get_inner_reference sounds like a risky change though. E.g. Richard's PR51750 change would need to be adjusted to match it.
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 Uros Bizjak ubizjak at gmail dot com changed: What|Removed |Added CC||rguenth at gcc dot gnu.org --- Comment #18 from Uros Bizjak ubizjak at gmail dot com 2012-01-26 08:20:27 UTC --- Adding richi to CC.
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994
--- Comment #19 from Richard Guenther rguenth at gcc dot gnu.org 2012-01-26
10:01:23 UTC ---
I agree, all callers of get_inner_reference need to cope with a negative
bitpos. Those that forward it unchecked to functions that expect an
unsigned bitpos are broken.
Thus I think fixing the prototypes is correct. If that exposes other
issues we have to fix them. The issue in extract_split_bit_field
is obviously the same - unsigned prototype and unsigned offset in
while (bitsdone bitsize)
{
unsigned HOST_WIDE_INT thissize;
rtx part, word;
unsigned HOST_WIDE_INT thispos;
unsigned HOST_WIDE_INT offset;
offset = (bitpos + bitsdone) / unit;
also
thispos = (bitpos + bitsdone) % unit;
might not be correct with a negative (bitpos + bitsdone).
extract_fixed_bit_field has the same prototype issue, so eventually we
want to simply account for them in the callers (if there are less).
Only memory operands may have a negative bitpos and those we should be
able to adjust via adjust_address (but by what amount?) to make bitpos
positive.
So you could say already the routines called from the get_inner_reference
callers should do that.
Eric, you should know this area the best - what do you recommend here?
[we could assert in the unsigned bitpos taking functions that the MSB
is not set on bitpos]
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #20 from Eric Botcazou ebotcazou at gcc dot gnu.org 2012-01-26 11:00:33 UTC --- Eric, you should know this area the best - what do you recommend here? [we could assert in the unsigned bitpos taking functions that the MSB is not set on bitpos] I agree that making get_inner_reference artificially return a non-zero poffset would most certainly be problematic as this would change the semantics. But it's also clear that the lower level bit-field manipulation routines aren't really prepared to deal with negative stuff. So I think that we shouldn't change the prototypes of these routines, but instead patch up callers that forward the values returned by get_inner_reference to these routines. Adding assertions in these routines could indeed help.
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994
Uros Bizjak ubizjak at gmail dot com changed:
What|Removed |Added
Attachment #26466|0 |1
is obsolete||
--- Comment #21 from Uros Bizjak ubizjak at gmail dot com 2012-01-26 18:40:24
UTC ---
Created attachment 26474
-- http://gcc.gnu.org/bugzilla/attachment.cgi?id=26474
Patch that adds asserts to {extract,store}_bit_field
This patch regressed following tests on x86_64-linux (gcc-46):
FAIL: gcc.dg/pr48335-8.c (internal compiler error)
FAIL: gcc.dg/pr48335-8.c (test for excess errors)
This shows that x86_64 is also not immune to negative bitpos problem.
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #22 from Uros Bizjak ubizjak at gmail dot com 2012-01-26 18:41:57 UTC --- (In reply to comment #20) I agree that making get_inner_reference artificially return a non-zero poffset would most certainly be problematic as this would change the semantics. But it's also clear that the lower level bit-field manipulation routines aren't really prepared to deal with negative stuff. So I think that we shouldn't change the prototypes of these routines, but instead patch up callers that forward the values returned by get_inner_reference to these routines. Adding assertions in these routines could indeed help. I have added these. But.. could you please take the fix to this problem further?
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #23 from Uros Bizjak ubizjak at gmail dot com 2012-01-26 18:51:00 UTC --- With a crosscompiler to alpha-linux-gnu, we can trigger both problems, one with preprocessed source, another with the testcase in latest attached patch: [uros@localhost testalpha]$ ~/gcc-build-alpha-46/gcc/cc1 -O2 -quiet pr51994.c pr51994.c: In function ‘test’: pr51994.c:17:3: internal compiler error: in extract_bit_field, at expmed.c:1701 Please submit a full bug report, with preprocessed source if appropriate. See http://gcc.gnu.org/bugs.html for instructions. [uros@localhost testalpha]$ ~/gcc-build-alpha-46/gcc/cc1 -O2 -quiet config.i config.c: In function ‘git_config_rename_section’: config.c:1533:16: internal compiler error: in store_bit_field, at expmed.c:839 Please submit a full bug report, with preprocessed source if appropriate. See http://gcc.gnu.org/bugs.html for instructions.
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 Eric Botcazou ebotcazou at gcc dot gnu.org changed: What|Removed |Added Status|NEW |ASSIGNED AssignedTo|unassigned at gcc dot |ebotcazou at gcc dot |gnu.org |gnu.org --- Comment #24 from Eric Botcazou ebotcazou at gcc dot gnu.org 2012-01-26 21:01:12 UTC --- Investigating.
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 Richard Guenther rguenth at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2012-01-25 CC||ebotcazou at gcc dot ||gnu.org Component|tree-optimization |middle-end Ever Confirmed|0 |1 --- Comment #1 from Richard Guenther rguenth at gcc dot gnu.org 2012-01-25 10:56:37 UTC --- Negative bitpos is fine - Ada uses that quite extensively and with MEM_REFs this just got more prominent. get_inner_reference is declared to return a _signed_ HOST_WIDE_INT bitpos for a reason. What should happen instead is that store_field needs to adjust the address to properly point before the bitfield for calling store_bit_field. Or the latter needs to take signed arguments for bitpos and do the adjustment itself. Does simply changing the store_bit_field[_1] prototype work for you?
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #2 from Eric Botcazou ebotcazou at gcc dot gnu.org 2012-01-25 11:15:09 UTC --- Negative bitpos is fine - Ada uses that quite extensively and with MEM_REFs this just got more prominent. get_inner_reference is declared to return a _signed_ HOST_WIDE_INT bitpos for a reason. Extensively is a bit of an overstatement, but Ada does use negative offsets. The recent story about them in build_ref_for_model shows that they can be problematic though. What should happen instead is that store_field needs to adjust the address to properly point before the bitfield for calling store_bit_field. Or the latter needs to take signed arguments for bitpos and do the adjustment itself. Does simply changing the store_bit_field[_1] prototype work for you? It might also be interesting to find out why we have a negative bitpos here.
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #3 from Uros Bizjak ubizjak at gmail dot com 2012-01-25 11:21:34 UTC --- Created attachment 26459 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=26459 Patch to fix function prototypes To my surprise, attached patch fixes all git failures. However, I have no idea if changing the prototypes as suggested by Richi is the right way to fix this.
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #4 from Uros Bizjak ubizjak at gmail dot com 2012-01-25 11:39:11 UTC --- (In reply to comment #2) What should happen instead is that store_field needs to adjust the address to properly point before the bitfield for calling store_bit_field. Or the latter needs to take signed arguments for bitpos and do the adjustment itself. Does simply changing the store_bit_field[_1] prototype work for you? It might also be interesting to find out why we have a negative bitpos here. gcc passes (buf - 1) to get_innter_reference, so it returns *pbitpos = -8.
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #5 from Richard Guenther rguenth at gcc dot gnu.org 2012-01-25 12:41:13 UTC --- (In reply to comment #3) Created attachment 26459 [details] Patch to fix function prototypes To my surprise, attached patch fixes all git failures. However, I have no idea if changing the prototypes as suggested by Richi is the right way to fix this. I think it is certainly obviously correct to do this. OTOH it may uncover latent issues (or not pass bootstrap due to signed/unsigned compares or so). I'd say give it bootstrap regtest on a fast machine and post this patch.
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994
--- Comment #6 from Richard Guenther rguenth at gcc dot gnu.org 2012-01-25
12:42:14 UTC ---
(In reply to comment #2)
Negative bitpos is fine - Ada uses that quite extensively and with MEM_REFs
this just got more prominent. get_inner_reference is declared to return
a _signed_ HOST_WIDE_INT bitpos for a reason.
Extensively is a bit of an overstatement, but Ada does use negative offsets.
The recent story about them in build_ref_for_model shows that they can be
problematic though.
What should happen instead is that store_field needs to adjust the address
to properly point before the bitfield for calling store_bit_field. Or the
latter needs to take signed arguments for bitpos and do the adjustment
itself.
Does simply changing the store_bit_field[_1] prototype work for you?
It might also be interesting to find out why we have a negative bitpos here.
It's as easy as doing
int foo(int *p)
{
return p[-1];
}
these days.
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994
--- Comment #7 from Uros Bizjak ubizjak at gmail dot com 2012-01-25 13:19:32
UTC ---
Testcase that crashes on alpha:
--cut here--
extern void abort (void);
char __attribute__((noinline))
test (int a)
{
char buf[] = 0123456789;
char *output = buf;
output += a;
output -= 1;
return output[0];
}
int main ()
{
if (test (2) != '1')
abort ();
return 0;
}
--cut here--
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #8 from Uros Bizjak ubizjak at gmail dot com 2012-01-25 13:33:43 UTC --- And the test in Comment #7 exposed the same problem in extract_bit_field co. #19 0x005801f4 in extract_bit_field (str_rtx=0x2e85b760, bitsize=46912560805760, bitnum=46912559843392, unsignedp=370, packedp=value optimized out, target=0x0, mode=QImode, tmode=QImode) ---Type return to continue, or q return to quit--- at ../../gcc-svn/branches/gcc-4_6-branch/gcc/expmed.c:1697 #20 0x0058aa12 in expand_expr_real_1 (exp=0x2e773870, target=0x2e85b700, tmode=value optimized out, modifier=value optimized out, alt_rtl=value optimized out) at ../../gcc-svn/branches/gcc-4_6-branch/gcc/expr.c:9380 What a can of worms...
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 Uros Bizjak ubizjak at gmail dot com changed: What|Removed |Added Attachment #26459|0 |1 is obsolete|| --- Comment #9 from Uros Bizjak ubizjak at gmail dot com 2012-01-25 14:28:10 UTC --- Created attachment 26462 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=26462 Patch to fix function prototypes, adds extract_bit_field
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994
--- Comment #10 from Uros Bizjak ubizjak at gmail dot com 2012-01-25 14:41:39
UTC ---
(In reply to comment #7)
Testcase that crashes on alpha:
Actually, the test in comment #7 exposed the problem, but was not 100% correct.
This one is:
--cut here--
#include string.h
extern void abort (void);
char __attribute__((noinline))
test (int a)
{
char buf[16];
char *output = buf;
strcpy (buf[0], 0123456789);
output += a;
output -= 1;
return output[0];
}
int main ()
{
if (test (2) != '1')
abort ();
return 0;
}
--cut here--
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 Jakub Jelinek jakub at gcc dot gnu.org changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #11 from Jakub Jelinek jakub at gcc dot gnu.org 2012-01-25 14:46:04 UTC --- Please avoid string.h, then it depends on the C library headers. Either provide your own strcpy prototype like you already do for abort, or use __builtin_strcpy?
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 Uros Bizjak ubizjak at gmail dot com changed: What|Removed |Added Attachment #26462|0 |1 is obsolete|| --- Comment #12 from Uros Bizjak ubizjak at gmail dot com 2012-01-25 18:17:25 UTC --- Created attachment 26466 -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=26466 WIP gcc-46 patch, includes other expmed.c store/extract bit functions This patch fixes regression at gcc.dg/pr48335-8.c on x86_64-linux from previous patch. However, some kind of unterminated loop is now triggered for this test on x86_64 target: (gdb) up #2 0x005b1a3d in extract_split_bit_field (op0=0x71340a80, bitsize=16, bitpos=-24, unsignedp=1) at ../../gcc-svn/branches/gcc-4_6-branch/gcc/expmed.c:1996 1996 word = operand_subword_force (op0, offset, GET_MODE (op0)); (gdb) up #3 0x005b1962 in extract_split_bit_field (op0=0x71340a80, bitsize=16, bitpos=-24, unsignedp=1) at ../../gcc-svn/branches/gcc-4_6-branch/gcc/expmed.c:2006 2006 part = extract_fixed_bit_field (word_mode, word, (gdb) up #4 0x005b1962 in extract_split_bit_field (op0=0x71340a80, bitsize=16, bitpos=-24, unsignedp=1) at ../../gcc-svn/branches/gcc-4_6-branch/gcc/expmed.c:2006 2006 part = extract_fixed_bit_field (word_mode, word, I was not able to fix it... definitelly for someone more experienced in this area of the compiler.
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #13 from Uros Bizjak ubizjak at gmail dot com 2012-01-25 18:39:38 UTC --- Fixing bit position to unsigned in headers is a No-Go. Too many parts of the compiler depends on unsigned bit positions - we can end with negative subreg indexes. Perhpaps the return of get_inner_reference can be adjusted to return equivalent negative offset expression instead of negative bit position?
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994
--- Comment #14 from Uros Bizjak ubizjak at gmail dot com 2012-01-25 19:44:03
UTC ---
(In reply to comment #13)
Perhpaps the return of get_inner_reference can be adjusted to return
equivalent
negative offset expression instead of negative bit position?
Like this:
Index: expr.c
===
--- expr.c (revision 183524)
+++ expr.c (working copy)
@@ -6300,6 +6300,22 @@ get_inner_reference (tree exp, HOST_WIDE_INT *pbit
*poffset = offset;
}
+ /* Negative bit positions are not allowed. */
+ if (*pbitpos 0)
+{
+ tree cst = build_int_cst (NULL_TREE, *pbitpos / BITS_PER_UNIT);
+
+ if (*poffset)
+ *poffset = fold_convert (sizetype,
+fold_build2 (PLUS_EXPR,
+ sizetype,
+ *poffset, cst));
+ else
+ *poffset = cst;
+
+ *pbitpos = 0;
+}
+
/* We can use BLKmode for a byte-aligned BLKmode bitfield. */
if (mode == VOIDmode
blkmode_bitfield
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #15 from Jakub Jelinek jakub at gcc dot gnu.org 2012-01-25 20:20:12 UTC --- Can't most of the callers of get_inner_reference cope with negative bitpos though? If so, perhaps only the caller or two in the expansion which doesn't should be adjusted.
[Bug middle-end/51994] [4.6/4.7 Regression] git-1.7.8.3 miscompiled due to negative bitpos from get_inner_reference
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51994 --- Comment #16 from Uros Bizjak ubizjak at gmail dot com 2012-01-26 07:57:10 UTC --- (In reply to comment #15) Can't most of the callers of get_inner_reference cope with negative bitpos though? If so, perhaps only the caller or two in the expansion which doesn't should be adjusted. I did look a bit for other uses, and negative offsets can result in subregs with negative index and shifts with negative shift constant (see i.e. fold-const.c, line 3454). So, I still think that infrastructure doesn't handle negative bit positions well.
