* [Bug tree-optimization/62035] [4.9/4.10 Regression] wrong code building libapache-mod-perl with -O1, works with -O1 -fno-tree-dse
2014-08-06 12:56 [Bug tree-optimization/62035] New: [4.9 Regresion] wrong code building libapache-mod-perl with -O1, works with -O1 -fno-tree-dse doko at gcc dot gnu.org
2014-08-07 15:44 ` [Bug tree-optimization/62035] [4.9/4.10 Regression] " wschmidt at gcc dot gnu.org
@ 2014-08-08 8:17 ` rguenth at gcc dot gnu.org
2014-08-08 8:18 ` rguenth at gcc dot gnu.org
2014-08-08 13:39 ` glisse at gcc dot gnu.org
3 siblings, 0 replies; 5+ messages in thread
From: rguenth at gcc dot gnu.org @ 2014-08-08 8:17 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62035
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |RESOLVED
Resolution|--- |INVALID
--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
I can reproduce the bad assembly with -O -g -fwrapv -fstack-protector -fPIC
--param ssp-buffer-size=4 --param ssp-buffer-size=4
The only dead store deleted in modperl_env_table_populate is
Deleted dead store 'sv = sv.5_31;
and that's ok because we have a clobber of 'sv' in this path.
<bb 7>:
_29 = _22->val;
sv.5_31 = Perl_newSVpv (my_perl_7(D), _29, 0);
sv = sv.5_31;
_33 = _22->key;
Perl_hv_common_key_len (my_perl_7(D), hv_10, _33, klen_26, 36, sv.5_31, 0);
_35 = _22->key;
Perl_sv_magic (my_perl_7(D), sv.5_31, 0B, 101, _35, klen_26);
# DEBUG svp => &sv
sv ={v} {CLOBBER};
<bb 8>:
# svp_2 = PHI <&sv(7), svp_28(6)>
# DEBUG svp => svp_2
_41 = my_perl_7(D)->Itainting;
if (_41 != 0)
goto <bb 9>;
else
goto <bb 10>;
<bb 9>:
_42 = *svp_2;
Perl_sv_magic (my_perl_7(D), _42, 0B, 116, 0B, 0);
but a use-after-free here through *svp_2. Prettified preprocessed souce
looks like
SV **svp = ((SV**) Perl_hv_common_key_len(my_perl,
(hv),(elts[i].key),(klen),((0)) ? (0x20 | 0x10) : 0x20,((void *)0),0));
if (svp) {
Perl_sv_setpv(my_perl, *svp,elts[i].val);
} else {
SV *sv = Perl_newSVpv(my_perl, elts[i].val,0);
(void)((SV**) Perl_hv_common_key_len(my_perl,
(hv),(elts[i].key),(klen),(0x04|0x20),(sv),((0))));
Perl_sv_magic(my_perl, sv,(SV *)((void *)0),'e',elts[i].key,klen);
svp = &sv;
}
if (0) modperl_trace(__func__, "$ENV{%s} = \"%s\";", elts[i].key,
elts[i].val);
(void)({
if(((my_perl->Itainting))){Perl_sv_magic(my_perl, (*svp),((void
*)0),'t',((void *)0),0);
so 'sv' is declared inside the 'else' but you make its address escape
through the 'svp' variable declared in the outer block.
Invalid.
A fix is to move the declaration of SV *sv up one block.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Bug tree-optimization/62035] [4.9/4.10 Regression] wrong code building libapache-mod-perl with -O1, works with -O1 -fno-tree-dse
2014-08-06 12:56 [Bug tree-optimization/62035] New: [4.9 Regresion] wrong code building libapache-mod-perl with -O1, works with -O1 -fno-tree-dse doko at gcc dot gnu.org
` (2 preceding siblings ...)
2014-08-08 8:18 ` rguenth at gcc dot gnu.org
@ 2014-08-08 13:39 ` glisse at gcc dot gnu.org
3 siblings, 0 replies; 5+ messages in thread
From: glisse at gcc dot gnu.org @ 2014-08-08 13:39 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62035
--- Comment #5 from Marc Glisse <glisse at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #4)
> Marc, do you think it's possible to warn about such "return of local
> addresses"?
It doesn't seem easy. Here are some random thoughts.
At the exit of a block, we can say that the address of sv becomes nonsense, but
we don't have an easy way to replace it with 0 or undefined. The front end
doesn't track the leaking address. In the middle-end, we don't know anymore
that &sv is meaningless. AFAIK, in SSA, all variables live for the whole
function, the clobber only tells us that the content is nonsense, but the
address itself can perfectly well be reused to write new content. So we are
left with trying to warn about the read-after-clobber.
bb 10 ends with:
# .MEM_37 = VDEF <.MEM_36>
svD.22441 ={v} {CLOBBER};
bb 11 contains:
# svp_2 = PHI <&svD.22441(10), svp_28(9)>
# .MEM_3 = PHI <.MEM_37(10), .MEM_40(9)>
if (_41 != 0)
goto <bb 12>;
and bb 12 starts with:
# VUSE <.MEM_3>
_42 = *svp_2;
It is possible to see from this that _42 will receive garbage if we take the
path 10 -> 11 -> 12. But that's going to be an awfully specific piece of code
(and we don't want to start a walk of vdefs every time something is
dereferenced). I haven't yet studied the passes that track values in memory, so
I don't know if they can be adapted. I think there are lower hanging fruits
than this though.
^ permalink raw reply [flat|nested] 5+ messages in thread