Re: [PATCH] Fix up compound literal handling in C FE (PR sanitizer/84307)

2018-02-26 Thread Jakub Jelinek
On Mon, Feb 26, 2018 at 11:00:48AM +0100, Martin Liška wrote:
> On 02/14/2018 11:03 PM, Joseph Myers wrote:
> > On Wed, 14 Feb 2018, Jakub Jelinek wrote:
> > 
> >> 2018-02-13  Jakub Jelinek  
> >>
> >>PR sanitizer/84340
> >>* c-decl.c (build_compound_literal): Call pushdecl (decl) even when
> >>it is not TREE_STATIC.
> >>* c-typeck.c (c_mark_addressable) : Mark
> >>not just the COMPOUND_LITERAL_EXPR node itself addressable, but also
> >>its COMPOUND_LITERAL_EXPR_DECL.
> > 
> > OK.
> > 
> 
> Jakub you probably forgot to install the patch. Can you please do it?

No, I was asking for ack for stage1, so it is waiting for GCC9.
This isn't really a regression and feels risky for GCC8 at this stage, might
break invalid programs that try to use compound literals after they go out
of scope.

Jakub


Re: [PATCH] Fix up compound literal handling in C FE (PR sanitizer/84307)

2018-02-26 Thread Martin Liška
On 02/26/2018 11:04 AM, Jakub Jelinek wrote:
> On Mon, Feb 26, 2018 at 11:00:48AM +0100, Martin Liška wrote:
>> On 02/14/2018 11:03 PM, Joseph Myers wrote:
>>> On Wed, 14 Feb 2018, Jakub Jelinek wrote:
>>>
 2018-02-13  Jakub Jelinek  

PR sanitizer/84340
* c-decl.c (build_compound_literal): Call pushdecl (decl) even when
it is not TREE_STATIC.
* c-typeck.c (c_mark_addressable) : Mark
not just the COMPOUND_LITERAL_EXPR node itself addressable, but also
its COMPOUND_LITERAL_EXPR_DECL.
>>>
>>> OK.
>>>
>>
>> Jakub you probably forgot to install the patch. Can you please do it?
> 
> No, I was asking for ack for stage1, so it is waiting for GCC9.
> This isn't really a regression and feels risky for GCC8 at this stage, might
> break invalid programs that try to use compound literals after they go out
> of scope.
> 
>   Jakub
> 

Ah, I see!

Thanks,
Martin


Re: [PATCH] Fix up compound literal handling in C FE (PR sanitizer/84307)

2018-02-26 Thread Martin Liška
On 02/14/2018 11:03 PM, Joseph Myers wrote:
> On Wed, 14 Feb 2018, Jakub Jelinek wrote:
> 
>> 2018-02-13  Jakub Jelinek  
>>
>>  PR sanitizer/84340
>>  * c-decl.c (build_compound_literal): Call pushdecl (decl) even when
>>  it is not TREE_STATIC.
>>  * c-typeck.c (c_mark_addressable) : Mark
>>  not just the COMPOUND_LITERAL_EXPR node itself addressable, but also
>>  its COMPOUND_LITERAL_EXPR_DECL.
> 
> OK.
> 

Jakub you probably forgot to install the patch. Can you please do it?
Should I create a new test-case which you mentioned in previous email?

Martin


Re: [PATCH] Fix up compound literal handling in C FE (PR sanitizer/84307)

2018-02-14 Thread Joseph Myers
On Wed, 14 Feb 2018, Jakub Jelinek wrote:

> 2018-02-13  Jakub Jelinek  
> 
>   PR sanitizer/84340
>   * c-decl.c (build_compound_literal): Call pushdecl (decl) even when
>   it is not TREE_STATIC.
>   * c-typeck.c (c_mark_addressable) : Mark
>   not just the COMPOUND_LITERAL_EXPR node itself addressable, but also
>   its COMPOUND_LITERAL_EXPR_DECL.

OK.

-- 
Joseph S. Myers
jos...@codesourcery.com


[PATCH] Fix up compound literal handling in C FE (PR sanitizer/84307)

2018-02-14 Thread Jakub Jelinek
On Tue, Feb 13, 2018 at 03:40:20PM +0100, Jakub Jelinek wrote:
> BTW, your testcase shows a more severe problem, that we actually don't
> handle compound literals correctly.
> 
> C99 says that:
> "If the compound literal occurs outside the body of a function, the object
> has static storage duration; otherwise, it has automatic storage duration
> associated with the enclosing block."
> but if we create an object with automatic storage duration, we don't
> actually put that object into the scope of the enclosing block, but of the
> enclosing function, which explains the weird ASAN_MARK UNPOISON present, but
> corresponding ASAN_MARK POISON not present.  The following testcase should
> IMHO FAIL with -fsanitize=address on the second bar call, but doesn't, even
> at -O0 without any DSE.  When optimizing we because of this don't emit
> CLOBBER stmts when the compound literal object goes out of scope, and with
> -fsanitize=address -fsanitize-address-use-after-scope we don't emit the
> POISON.

Here is the full patch that passed bootstrap/regtest on x86_64-linux and
i686-linux, ok for stage1?

The mark_addressable part is needed to fix
gcc.c-torture/compile/compound-literal-3.c, where we marked the underlying
automatic temporary of the compound literal as addressable too late and
the gimplifier already set DECL_GIMPLE_REG_P on it when seeing it in the
block scope.

2018-02-13  Jakub Jelinek  

PR sanitizer/84340
* c-decl.c (build_compound_literal): Call pushdecl (decl) even when
it is not TREE_STATIC.
* c-typeck.c (c_mark_addressable) : Mark
not just the COMPOUND_LITERAL_EXPR node itself addressable, but also
its COMPOUND_LITERAL_EXPR_DECL.

--- gcc/c/c-decl.c.jj   2018-02-13 21:21:35.447978191 +0100
+++ gcc/c/c-decl.c  2018-02-13 22:13:02.030148159 +0100
@@ -5348,6 +5348,8 @@ build_compound_literal (location_t loc,
   pushdecl (decl);
   rest_of_decl_compilation (decl, 1, 0);
 }
+  else
+pushdecl (decl);
 
   if (non_const)
 {
--- gcc/c/c-typeck.c.jj 2018-02-10 00:15:36.398163493 +0100
+++ gcc/c/c-typeck.c2018-02-13 22:24:06.294104876 +0100
@@ -4821,6 +4821,10 @@ c_mark_addressable (tree exp, bool array
break;
 
   case COMPOUND_LITERAL_EXPR:
+   TREE_ADDRESSABLE (x) = 1;
+   TREE_ADDRESSABLE (COMPOUND_LITERAL_EXPR_DECL (x)) = 1;
+   return true;
+
   case CONSTRUCTOR:
TREE_ADDRESSABLE (x) = 1;
return true;


Jakub