[PATCH][PR lto/79061] Fix LTO plus ASAN fails with "AddressSanitizer: initialization-order-fiasco".

[PATCH][PR lto/79061] Fix LTO plus ASAN fails with "AddressSanitizer: initialization-order-fiasco".

[Maxim Ostapenko]

[-- Attachment #1: Type: text/plain, Size: 1744 bytes --]

Hi,

as was figured out in PR LTO + ASan raises false initialization order 
fiasco alarm due to in LTO case main_input_filename doesn't match module 
name passed to __asan_before_dynamic_init.
Following Jakub's suggestion I used TRANSLATION_UNIT_DECL for 
corresponding globals to overcome this issue (I needed to create a 
source location for each TRANSLATION_UNIT_DECL).
However, when testing, I hit on a nasty issue: for some reason 
linemap_ordinary_map_lookup, called from lto_output_location for given 
TRANSLATION_UNIT_DECL, hit an assert:

[...]
   linemap_assert (line >= MAP_START_LOCATION (result));
   return result;
}

due to line == 2.

After some investigation I noticed that source locations are propagated 
through location cache that can be partially invalidated by 
lto_location_cache::revert_location_cache call. And that was my case: 
after adding source location for TRANSLATION_UNIT_DECL into location 
cache, it was reverted by calling 
lto_location_cache::revert_location_cache from unify_scc before it was 
accepted:

static void
lto_read_decls (struct lto_file_decl_data *decl_data, const void *data,
                 vec<ld_plugin_symbol_resolution_t> resolutions)
{
[...]
           /* Try to unify the SCC with already existing ones.  */
           if (!flag_ltrans
               && unify_scc (data_in, from,
                             len, scc_entry_len, scc_hash))
             continue;

For now I can overcome it by calling 
location_cache.accept_location_cache for TRANSLATION_UNIT_DECL, but I 
wonder if more reliable fix is possible.

Attached patch fixes the issue mentioned in PR and passes regression 
testing and LTO bootstrap on x86_64-unknown-linux-gnu.
Could you please take a look on it?

-Maxim

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pr79061-1.diff --]
[-- Type: text/x-diff; name="pr79061-1.diff", Size: 4681 bytes --]

gcc/lto/ChangeLog:

2017-01-17  Maxim Ostapenko  <m.ostapenko@samsung.com>

	* lto.c (lto_read_decls): accept location cache for
	TRANSLATION_UNIT_DECL.

gcc/testsuite/ChangeLog:

2017-01-17  Maxim Ostapenko  <m.ostapenko@samsung.com>

	* gcc.dg/cpp/mi1.c: Adjust testcase.
	* gcc.dg/pch/cpp-3.c: Likewise.

gcc/ChangeLog:

2017-01-17  Maxim Ostapenko  <m.ostapenko@samsung.com>

	* asan.c (get_translation_unit_decl): New function.
	(asan_add_global): Extract modules file name from globals
	TRANSLATION_UNIT_DECL in lto mode.
	* tree.c (build_translation_unit_decl): Add source location for newly
	built TRANSLATION_UNIT_DECL.

diff --git a/gcc/asan.c b/gcc/asan.c
index 7450044..9a59fe4 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -2372,6 +2372,22 @@ asan_needs_odr_indicator_p (tree decl)
 	  && TREE_PUBLIC (decl));
 }
 
+/* For given DECL return its corresponding TRANSLATION_UNIT_DECL.  */
+
+static const_tree
+get_translation_unit_decl (tree decl)
+{
+  const_tree context = decl;
+  while (context && TREE_CODE (context) != TRANSLATION_UNIT_DECL)
+    {
+      if (TREE_CODE (context) == BLOCK)
+	context = BLOCK_SUPERCONTEXT (context);
+      else
+	context = get_containing_scope (context);
+    }
+  return context;
+}
+
 /* Append description of a single global DECL into vector V.
    TYPE is __asan_global struct type as returned by asan_global_struct.  */
 
@@ -2391,7 +2407,14 @@ asan_add_global (tree decl, tree type, vec<constructor_elt, va_gc> *v)
     pp_string (&asan_pp, "<unknown>");
   str_cst = asan_pp_string (&asan_pp);
 
-  pp_string (&module_name_pp, main_input_filename);
+  const char *filename = main_input_filename;
+  if (in_lto_p)
+    {
+      const_tree translation_unit_decl = get_translation_unit_decl (decl);
+      if (translation_unit_decl)
+	filename = DECL_SOURCE_FILE (translation_unit_decl);
+    }
+  pp_string (&module_name_pp, filename);
   module_name_cst = asan_pp_string (&module_name_pp);
 
   if (asan_needs_local_alias (decl))
diff --git a/gcc/lto/lto.c b/gcc/lto/lto.c
index d77d85d..c65e7cd 100644
--- a/gcc/lto/lto.c
+++ b/gcc/lto/lto.c
@@ -1707,7 +1707,13 @@ lto_read_decls (struct lto_file_decl_data *decl_data, const void *data,
 	      && (TREE_CODE (first) == IDENTIFIER_NODE
 		  || TREE_CODE (first) == INTEGER_CST
 		  || TREE_CODE (first) == TRANSLATION_UNIT_DECL))
-	    continue;
+	    {
+	      /* For TRANSLATION_UNIT_DECL we need to accept location cache now
+	         to avoid possible reverting during following unify_scc call.  */
+	      if (TREE_CODE (first) == TRANSLATION_UNIT_DECL)
+		data_in->location_cache.accept_location_cache ();
+	      continue;
+	    }
 
 	  /* Try to unify the SCC with already existing ones.  */
 	  if (!flag_ltrans
diff --git a/gcc/testsuite/gcc.dg/cpp/mi1.c b/gcc/testsuite/gcc.dg/cpp/mi1.c
index 0cfedad..9817431 100644
--- a/gcc/testsuite/gcc.dg/cpp/mi1.c
+++ b/gcc/testsuite/gcc.dg/cpp/mi1.c
@@ -13,7 +13,7 @@
 
 /* { dg-do compile }
    { dg-options "-H" }
-   { dg-message "mi1c\.h\n\[^\n\]*mi1cc\.h\n\[^\n\]*mi1nd\.h\n\[^\n\]*mi1ndp\.h\n\[^\n\]*mi1x\.h" "redundant include check" { target *-*-* } 0 } */
+   { dg-message "mi1c\.h\n\[^\n\]*mi1cc\.h\n\[^\n\]*mi1nd\.h\n\[^\n\]*mi1ndp\.h\n\[^\n\]*mi1x\.h\n\[^\n\]*mi1\.c" "redundant include check" { target *-*-* } 0 } */
 
 #include "mi1c.h"
 #include "mi1c.h"
diff --git a/gcc/testsuite/gcc.dg/pch/cpp-3.c b/gcc/testsuite/gcc.dg/pch/cpp-3.c
index 25b5ca4..d635706 100644
--- a/gcc/testsuite/gcc.dg/pch/cpp-3.c
+++ b/gcc/testsuite/gcc.dg/pch/cpp-3.c
@@ -1,7 +1,7 @@
 /* PR preprocessor/36649 */
 /* { dg-do compile } */
 /* { dg-options "-H -I." } */
-/* { dg-message "cpp-3.h\[^\n\]*(\n\[^\n\]*cpp-3.c)?\n\[^\n\]*cpp-3a.h\n\[^\n\]*cpp-3b.h" "" { target *-*-* } 0 } */
+/* { dg-message "cpp-3.h\[^\n\]*(\n\[^\n\]*cpp-3.c)?\n\[^\n\]*cpp-3a.h\n\[^\n\]*cpp-3b.h\n\[^\n\]*cpp-3.c" "" { target *-*-* } 0 } */
 
 #include "cpp-3.h"
 #include "cpp-3a.h"
diff --git a/gcc/tree.c b/gcc/tree.c
index cffa36d..59fe8d4 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -4758,7 +4758,10 @@ vec<tree, va_gc> *all_translation_units;
 tree
 build_translation_unit_decl (tree name)
 {
-  tree tu = build_decl (UNKNOWN_LOCATION, TRANSLATION_UNIT_DECL,
+  linemap_add (line_table, LC_ENTER, false, main_input_filename, 1);
+  location_t loc = linemap_line_start (line_table, 1, 0);
+  linemap_add (line_table, LC_LEAVE, false, NULL, 0);
+  tree tu = build_decl (loc, TRANSLATION_UNIT_DECL,
 			name, NULL_TREE);
   TRANSLATION_UNIT_LANGUAGE (tu) = lang_hooks.name;
   vec_safe_push (all_translation_units, tu);

Re: [PATCH][PR lto/79061] Fix LTO plus ASAN fails with "AddressSanitizer: initialization-order-fiasco".

[Richard Biener]

On Wed, 18 Jan 2017, Maxim Ostapenko wrote:

> Hi,
> 
> as was figured out in PR LTO + ASan raises false initialization order fiasco
> alarm due to in LTO case main_input_filename doesn't match module name passed
> to __asan_before_dynamic_init.
> Following Jakub's suggestion I used TRANSLATION_UNIT_DECL for corresponding
> globals to overcome this issue (I needed to create a source location for each
> TRANSLATION_UNIT_DECL).
> However, when testing, I hit on a nasty issue: for some reason
> linemap_ordinary_map_lookup, called from lto_output_location for given
> TRANSLATION_UNIT_DECL, hit an assert:
> 
> [...]
>   linemap_assert (line >= MAP_START_LOCATION (result));
>   return result;
> }
> 
> due to line == 2.
> 
> After some investigation I noticed that source locations are propagated
> through location cache that can be partially invalidated by
> lto_location_cache::revert_location_cache call. And that was my case: after
> adding source location for TRANSLATION_UNIT_DECL into location cache, it was
> reverted by calling lto_location_cache::revert_location_cache from unify_scc
> before it was accepted:
> 
> static void
> lto_read_decls (struct lto_file_decl_data *decl_data, const void *data,
>                 vec<ld_plugin_symbol_resolution_t> resolutions)
> {
> [...]
>           /* Try to unify the SCC with already existing ones.  */
>           if (!flag_ltrans
>               && unify_scc (data_in, from,
>                             len, scc_entry_len, scc_hash))
>             continue;
> 
> For now I can overcome it by calling location_cache.accept_location_cache for
> TRANSLATION_UNIT_DECL, but I wonder if more reliable fix is possible.
> 
> Attached patch fixes the issue mentioned in PR and passes regression testing
> and LTO bootstrap on x86_64-unknown-linux-gnu.
> Could you please take a look on it?

Looks good to me (aka, OK).

Thanks,
Richard.

> -Maxim
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Re: [PATCH][PR lto/79061] Fix LTO plus ASAN fails with "AddressSanitizer: initialization-order-fiasco".

[Maxim Ostapenko]

On 18/01/17 16:00, Richard Biener wrote:
> On Wed, 18 Jan 2017, Maxim Ostapenko wrote:
>
>> Hi,
>>
>> as was figured out in PR LTO + ASan raises false initialization order fiasco
>> alarm due to in LTO case main_input_filename doesn't match module name passed
>> to __asan_before_dynamic_init.
>> Following Jakub's suggestion I used TRANSLATION_UNIT_DECL for corresponding
>> globals to overcome this issue (I needed to create a source location for each
>> TRANSLATION_UNIT_DECL).
>> However, when testing, I hit on a nasty issue: for some reason
>> linemap_ordinary_map_lookup, called from lto_output_location for given
>> TRANSLATION_UNIT_DECL, hit an assert:
>>
>> [...]
>>    linemap_assert (line >= MAP_START_LOCATION (result));
>>    return result;
>> }
>>
>> due to line == 2.
>>
>> After some investigation I noticed that source locations are propagated
>> through location cache that can be partially invalidated by
>> lto_location_cache::revert_location_cache call. And that was my case: after
>> adding source location for TRANSLATION_UNIT_DECL into location cache, it was
>> reverted by calling lto_location_cache::revert_location_cache from unify_scc
>> before it was accepted:
>>
>> static void
>> lto_read_decls (struct lto_file_decl_data *decl_data, const void *data,
>>                  vec<ld_plugin_symbol_resolution_t> resolutions)
>> {
>> [...]
>>            /* Try to unify the SCC with already existing ones.  */
>>            if (!flag_ltrans
>>                && unify_scc (data_in, from,
>>                              len, scc_entry_len, scc_hash))
>>              continue;
>>
>> For now I can overcome it by calling location_cache.accept_location_cache for
>> TRANSLATION_UNIT_DECL, but I wonder if more reliable fix is possible.
>>
>> Attached patch fixes the issue mentioned in PR and passes regression testing
>> and LTO bootstrap on x86_64-unknown-linux-gnu.
>> Could you please take a look on it?
> Looks good to me (aka, OK).

Thanks, applied as r244581. Is it OK to commit the patch on branches if 
no new errors pop up in the next few days?

-Maxim

>
> Thanks,
> Richard.
>
>> -Maxim
>>

Re: [PATCH][PR lto/79061] Fix LTO plus ASAN fails with "AddressSanitizer: initialization-order-fiasco".

[Richard Biener]

On January 18, 2017 5:17:12 PM GMT+01:00, Maxim Ostapenko <m.ostapenko@samsung.com> wrote:
>On 18/01/17 16:00, Richard Biener wrote:
>> On Wed, 18 Jan 2017, Maxim Ostapenko wrote:
>>
>>> Hi,
>>>
>>> as was figured out in PR LTO + ASan raises false initialization
>order fiasco
>>> alarm due to in LTO case main_input_filename doesn't match module
>name passed
>>> to __asan_before_dynamic_init.
>>> Following Jakub's suggestion I used TRANSLATION_UNIT_DECL for
>corresponding
>>> globals to overcome this issue (I needed to create a source location
>for each
>>> TRANSLATION_UNIT_DECL).
>>> However, when testing, I hit on a nasty issue: for some reason
>>> linemap_ordinary_map_lookup, called from lto_output_location for
>given
>>> TRANSLATION_UNIT_DECL, hit an assert:
>>>
>>> [...]
>>>    linemap_assert (line >= MAP_START_LOCATION (result));
>>>    return result;
>>> }
>>>
>>> due to line == 2.
>>>
>>> After some investigation I noticed that source locations are
>propagated
>>> through location cache that can be partially invalidated by
>>> lto_location_cache::revert_location_cache call. And that was my
>case: after
>>> adding source location for TRANSLATION_UNIT_DECL into location
>cache, it was
>>> reverted by calling lto_location_cache::revert_location_cache from
>unify_scc
>>> before it was accepted:
>>>
>>> static void
>>> lto_read_decls (struct lto_file_decl_data *decl_data, const void
>*data,
>>>                  vec<ld_plugin_symbol_resolution_t> resolutions)
>>> {
>>> [...]
>>>            /* Try to unify the SCC with already existing ones.  */
>>>            if (!flag_ltrans
>>>                && unify_scc (data_in, from,
>>>                              len, scc_entry_len, scc_hash))
>>>              continue;
>>>
>>> For now I can overcome it by calling
>location_cache.accept_location_cache for
>>> TRANSLATION_UNIT_DECL, but I wonder if more reliable fix is
>possible.
>>>
>>> Attached patch fixes the issue mentioned in PR and passes regression
>testing
>>> and LTO bootstrap on x86_64-unknown-linux-gnu.
>>> Could you please take a look on it?
>> Looks good to me (aka, OK).
>
>Thanks, applied as r244581. Is it OK to commit the patch on branches if
>
>no new errors pop up in the next few days?

Yes.

Richard.
>
>-Maxim
>
>>
>> Thanks,
>> Richard.
>>
>>> -Maxim
>>>

Re: [PATCH][PR lto/79061] Fix LTO plus ASAN fails with "AddressSanitizer: initialization-order-fiasco".

[Richard Biener]

On Wed, 18 Jan 2017, Maxim Ostapenko wrote:

> On 18/01/17 16:00, Richard Biener wrote:
> > On Wed, 18 Jan 2017, Maxim Ostapenko wrote:
> > 
> > > Hi,
> > > 
> > > as was figured out in PR LTO + ASan raises false initialization order
> > > fiasco
> > > alarm due to in LTO case main_input_filename doesn't match module name
> > > passed
> > > to __asan_before_dynamic_init.
> > > Following Jakub's suggestion I used TRANSLATION_UNIT_DECL for
> > > corresponding
> > > globals to overcome this issue (I needed to create a source location for
> > > each
> > > TRANSLATION_UNIT_DECL).
> > > However, when testing, I hit on a nasty issue: for some reason
> > > linemap_ordinary_map_lookup, called from lto_output_location for given
> > > TRANSLATION_UNIT_DECL, hit an assert:
> > > 
> > > [...]
> > >    linemap_assert (line >= MAP_START_LOCATION (result));
> > >    return result;
> > > }
> > > 
> > > due to line == 2.
> > > 
> > > After some investigation I noticed that source locations are propagated
> > > through location cache that can be partially invalidated by
> > > lto_location_cache::revert_location_cache call. And that was my case:
> > > after
> > > adding source location for TRANSLATION_UNIT_DECL into location cache, it
> > > was
> > > reverted by calling lto_location_cache::revert_location_cache from
> > > unify_scc
> > > before it was accepted:
> > > 
> > > static void
> > > lto_read_decls (struct lto_file_decl_data *decl_data, const void *data,
> > >                  vec<ld_plugin_symbol_resolution_t> resolutions)
> > > {
> > > [...]
> > >            /* Try to unify the SCC with already existing ones.  */
> > >            if (!flag_ltrans
> > >                && unify_scc (data_in, from,
> > >                              len, scc_entry_len, scc_hash))
> > >              continue;
> > > 
> > > For now I can overcome it by calling location_cache.accept_location_cache
> > > for
> > > TRANSLATION_UNIT_DECL, but I wonder if more reliable fix is possible.
> > > 
> > > Attached patch fixes the issue mentioned in PR and passes regression
> > > testing
> > > and LTO bootstrap on x86_64-unknown-linux-gnu.
> > > Could you please take a look on it?
> > Looks good to me (aka, OK).
> 
> Thanks, applied as r244581. Is it OK to commit the patch on branches if no new
> errors pop up in the next few days?

So this regresses compile-time by 100% on some fortran cases.  Doing the
linemap operation in build_translation_unit_decl confuses the linemap
code too much it seems.

Please revert this (and not backport it) for now.

A better place to store the filename would be eventually the DECL_NAME
of it or a new field alongside TRANSLATION_UNIT_DECL_LANGUAGE.

Thanks,
Richard.

> -Maxim
> 
> > 
> > Thanks,
> > Richard.
> > 
> > > -Maxim
> > > 
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Re: [PATCH][PR lto/79061] Fix LTO plus ASAN fails with "AddressSanitizer: initialization-order-fiasco".

[Maxim Ostapenko]

On 23/01/17 12:03, Richard Biener wrote:
> On Wed, 18 Jan 2017, Maxim Ostapenko wrote:
>
>> On 18/01/17 16:00, Richard Biener wrote:
>>> On Wed, 18 Jan 2017, Maxim Ostapenko wrote:
>>>
>>>> Hi,
>>>>
>>>> as was figured out in PR LTO + ASan raises false initialization order
>>>> fiasco
>>>> alarm due to in LTO case main_input_filename doesn't match module name
>>>> passed
>>>> to __asan_before_dynamic_init.
>>>> Following Jakub's suggestion I used TRANSLATION_UNIT_DECL for
>>>> corresponding
>>>> globals to overcome this issue (I needed to create a source location for
>>>> each
>>>> TRANSLATION_UNIT_DECL).
>>>> However, when testing, I hit on a nasty issue: for some reason
>>>> linemap_ordinary_map_lookup, called from lto_output_location for given
>>>> TRANSLATION_UNIT_DECL, hit an assert:
>>>>
>>>> [...]
>>>>     linemap_assert (line >= MAP_START_LOCATION (result));
>>>>     return result;
>>>> }
>>>>
>>>> due to line == 2.
>>>>
>>>> After some investigation I noticed that source locations are propagated
>>>> through location cache that can be partially invalidated by
>>>> lto_location_cache::revert_location_cache call. And that was my case:
>>>> after
>>>> adding source location for TRANSLATION_UNIT_DECL into location cache, it
>>>> was
>>>> reverted by calling lto_location_cache::revert_location_cache from
>>>> unify_scc
>>>> before it was accepted:
>>>>
>>>> static void
>>>> lto_read_decls (struct lto_file_decl_data *decl_data, const void *data,
>>>>                   vec<ld_plugin_symbol_resolution_t> resolutions)
>>>> {
>>>> [...]
>>>>             /* Try to unify the SCC with already existing ones.  */
>>>>             if (!flag_ltrans
>>>>                 && unify_scc (data_in, from,
>>>>                               len, scc_entry_len, scc_hash))
>>>>               continue;
>>>>
>>>> For now I can overcome it by calling location_cache.accept_location_cache
>>>> for
>>>> TRANSLATION_UNIT_DECL, but I wonder if more reliable fix is possible.
>>>>
>>>> Attached patch fixes the issue mentioned in PR and passes regression
>>>> testing
>>>> and LTO bootstrap on x86_64-unknown-linux-gnu.
>>>> Could you please take a look on it?
>>> Looks good to me (aka, OK).
>> Thanks, applied as r244581. Is it OK to commit the patch on branches if no new
>> errors pop up in the next few days?
> So this regresses compile-time by 100% on some fortran cases.  Doing the
> linemap operation in build_translation_unit_decl confuses the linemap
> code too much it seems.
>
> Please revert this (and not backport it) for now.

Done with r244773, sorry about that :-( . We'll probably need to reopen 
the bug, right?

-Maxim

>
> A better place to store the filename would be eventually the DECL_NAME
> of it or a new field alongside TRANSLATION_UNIT_DECL_LANGUAGE.
>
> Thanks,
> Richard.
>
>> -Maxim
>>
>>> Thanks,
>>> Richard.
>>>
>>>> -Maxim
>>>>
>>

Re: [PATCH][PR lto/79061] Fix LTO plus ASAN fails with "AddressSanitizer: initialization-order-fiasco".

[Jakub Jelinek]

On Mon, Jan 23, 2017 at 12:14:21PM +0300, Maxim Ostapenko wrote:
> > Please revert this (and not backport it) for now.
> 
> Done with r244773, sorry about that :-( . We'll probably need to reopen the
> bug, right?

Sure.

	Jakub

[PATCH v2][PR lto/79061] Fix LTO plus ASAN fails with "AddressSanitizer: initialization-order-fiasco".

[Maxim Ostapenko]

[-- Attachment #1: Type: text/plain, Size: 519 bytes --]

Hi,

as Richard pointed out in previous mail, v1 patch regresses compile-time 
by 100% on some fortran cases (PR 79165).
Doing the linemap operation in build_translation_unit_decl confuses the 
linemap code too much, thus we propagate main_input_filename through 
DECL_NAME of TRANSLATION_UNIT_DECL instead of creating new source location.
OK to install if regtest and bootstrap complete successfully? I've also 
checked that new patch doesn't introduce compile-time regression on 
aermod.f90 testcase.

Thanks,
-Maxim

[-- Attachment #2: pr79061-2.diff --]
[-- Type: text/x-diff, Size: 4690 bytes --]

gcc/fortran/ChangeLog:

2017-01-24  Maxim Ostapenko  <m.ostapenko@samsung.com>

	* f95-lang.c (gfc_create_decls): Include stringpool.h.
	Pass main_input_filename to build_translation_unit_decl.

gcc/ada/ChangeLog:

2017-01-24  Maxim Ostapenko  <m.ostapenko@samsung.com>

	* gcc-interface/utils.c (get_global_context): pass main_input_filename
	to build_translation_unit_decl.

gcc/c/ChangeLog:

2017-01-24  Maxim Ostapenko  <m.ostapenko@samsung.com>

	* c-decl.c (pop_scope): pass main_input_filename to
	build_translation_unit_decl.

gcc/cp/ChangeLog:

2017-01-24  Maxim Ostapenko  <m.ostapenko@samsung.com>

	* decl.c (cxx_init_decl_processing): pass main_input_filename
	to build_translation_unit_decl.

gcc/ChangeLog:

2017-01-24  Maxim Ostapenko  <m.ostapenko@samsung.com>

	* asan.c (get_translation_unit_decl): New function.
	(asan_add_global): Extract modules file name from globals
	TRANSLATION_UNIT_DECL name.

diff --git a/gcc/ada/gcc-interface/utils.c b/gcc/ada/gcc-interface/utils.c
index 0ae381f..3cda631 100644
--- a/gcc/ada/gcc-interface/utils.c
+++ b/gcc/ada/gcc-interface/utils.c
@@ -666,7 +666,8 @@ get_global_context (void)
 {
   if (!global_context)
     {
-      global_context = build_translation_unit_decl (NULL_TREE);
+      global_context
+	= build_translation_unit_decl (get_identifier (main_input_filename));
       debug_hooks->register_main_translation_unit (global_context);
     }
 
diff --git a/gcc/asan.c b/gcc/asan.c
index 486ebfd..e1475bd 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -2373,6 +2373,22 @@ asan_needs_odr_indicator_p (tree decl)
 	  && TREE_PUBLIC (decl));
 }
 
+/* For given DECL return its corresponding TRANSLATION_UNIT_DECL.  */
+
+static const_tree
+get_translation_unit_decl (tree decl)
+{
+  const_tree context = decl;
+  while (context && TREE_CODE (context) != TRANSLATION_UNIT_DECL)
+    {
+      if (TREE_CODE (context) == BLOCK)
+	context = BLOCK_SUPERCONTEXT (context);
+      else
+	context = get_containing_scope (context);
+    }
+  return context;
+}
+
 /* Append description of a single global DECL into vector V.
    TYPE is __asan_global struct type as returned by asan_global_struct.  */
 
@@ -2392,7 +2408,14 @@ asan_add_global (tree decl, tree type, vec<constructor_elt, va_gc> *v)
     pp_string (&asan_pp, "<unknown>");
   str_cst = asan_pp_string (&asan_pp);
 
-  pp_string (&module_name_pp, main_input_filename);
+  const char *filename = main_input_filename;
+  if (in_lto_p)
+    {
+      const_tree translation_unit_decl = get_translation_unit_decl (decl);
+      if (translation_unit_decl)
+	filename = IDENTIFIER_POINTER (DECL_NAME (translation_unit_decl));
+    }
+  pp_string (&module_name_pp, filename);
   module_name_cst = asan_pp_string (&module_name_pp);
 
   if (asan_needs_local_alias (decl))
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 2f91e70..32edacc 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -1177,7 +1177,8 @@ pop_scope (void)
     context = current_function_decl;
   else if (scope == file_scope)
     {
-      tree file_decl = build_translation_unit_decl (NULL_TREE);
+      tree file_decl
+	= build_translation_unit_decl (get_identifier (main_input_filename));
       context = file_decl;
       debug_hooks->register_main_translation_unit (file_decl);
     }
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 792ebcc..af74dcd 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -4071,7 +4071,8 @@ cxx_init_decl_processing (void)
   gcc_assert (global_namespace == NULL_TREE);
   global_namespace = build_lang_decl (NAMESPACE_DECL, global_scope_name,
 				      void_type_node);
-  DECL_CONTEXT (global_namespace) = build_translation_unit_decl (NULL_TREE);
+  DECL_CONTEXT (global_namespace)
+    = build_translation_unit_decl (get_identifier (main_input_filename));
   debug_hooks->register_main_translation_unit
     (DECL_CONTEXT (global_namespace));
   TREE_PUBLIC (global_namespace) = 1;
diff --git a/gcc/fortran/f95-lang.c b/gcc/fortran/f95-lang.c
index 98ef837..44bd8dc 100644
--- a/gcc/fortran/f95-lang.c
+++ b/gcc/fortran/f95-lang.c
@@ -30,6 +30,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree.h"
 #include "gfortran.h"
 #include "trans.h"
+#include "stringpool.h"
 #include "diagnostic.h" /* For errorcount/warningcount */
 #include "langhooks.h"
 #include "langhooks-def.h"
@@ -190,7 +191,8 @@ gfc_create_decls (void)
   gfc_init_constants ();
 
   /* Build our translation-unit decl.  */
-  current_translation_unit = build_translation_unit_decl (NULL_TREE);
+  current_translation_unit
+    = build_translation_unit_decl (get_identifier (main_input_filename));
   debug_hooks->register_main_translation_unit (current_translation_unit);
 }
 

Re: [PATCH v2][PR lto/79061] Fix LTO plus ASAN fails with "AddressSanitizer: initialization-order-fiasco".

[Richard Biener]

On Tue, 24 Jan 2017, Maxim Ostapenko wrote:

> Hi,
> 
> as Richard pointed out in previous mail, v1 patch regresses compile-time by
> 100% on some fortran cases (PR 79165).
> Doing the linemap operation in build_translation_unit_decl confuses the
> linemap code too much, thus we propagate main_input_filename through DECL_NAME
> of TRANSLATION_UNIT_DECL instead of creating new source location.
> OK to install if regtest and bootstrap complete successfully? I've also
> checked that new patch doesn't introduce compile-time regression on aermod.f90
> testcase.

+      const_tree translation_unit_decl = get_translation_unit_decl 
(decl);
+      if (translation_unit_decl)
+       filename = IDENTIFIER_POINTER (DECL_NAME (translation_unit_decl));

please also check DECL_NAME is not NULL.

Ok with that change.

Thanks,
Richard.
 
> Thanks,
> -Maxim
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Re: [PATCH v2][PR lto/79061] Fix LTO plus ASAN fails with "AddressSanitizer: initialization-order-fiasco".

[Jakub Jelinek]

On Tue, Jan 24, 2017 at 01:10:46PM +0300, Maxim Ostapenko wrote:
> gcc/fortran/ChangeLog:
> 
> 2017-01-24  Maxim Ostapenko  <m.ostapenko@samsung.com>
> 
> 	* f95-lang.c (gfc_create_decls): Include stringpool.h.
> 	Pass main_input_filename to build_translation_unit_decl.
> 
> gcc/ada/ChangeLog:
> 
> 2017-01-24  Maxim Ostapenko  <m.ostapenko@samsung.com>
> 
> 	* gcc-interface/utils.c (get_global_context): pass main_input_filename
> 	to build_translation_unit_decl.

s/pass/Pass/, after ): ChangeLog entries start with capital letter.

> gcc/c/ChangeLog:
> 
> 2017-01-24  Maxim Ostapenko  <m.ostapenko@samsung.com>
> 
> 	* c-decl.c (pop_scope): pass main_input_filename to
> 	build_translation_unit_decl.
> 
> gcc/cp/ChangeLog:
> 
> 2017-01-24  Maxim Ostapenko  <m.ostapenko@samsung.com>
> 
> 	* decl.c (cxx_init_decl_processing): pass main_input_filename
> 	to build_translation_unit_decl.

Likewise 2x.
> 
> gcc/ChangeLog:
> 
> 2017-01-24  Maxim Ostapenko  <m.ostapenko@samsung.com>
> 
> 	* asan.c (get_translation_unit_decl): New function.
> 	(asan_add_global): Extract modules file name from globals
> 	TRANSLATION_UNIT_DECL name.

Ok for trunk with that.

	Jakub

[PATCH v3][PR lto/79061] Fix LTO plus ASAN fails with "AddressSanitizer: initialization-order-fiasco".

[Maxim Ostapenko]

[-- Attachment #1: Type: text/plain, Size: 715 bytes --]

Hi,

as was figured out in PR, using DECL_NAME (TRANSLATION_UNIT_DECL) does 
not always give us a correct module name in LTO mode because e.g. 
DECL_CONTEXT of some variables can be NAMESPACE_DECL and LTO merges 
NAMESPACE_DECLs. The easiest fix is just to disable the initialization 
order checking altogether for LTO by forcing dynamically_initialized = 0 
in LTO for now but we'll probably want to restore initialization order 
fiasco detection in the future (e.g. for GCC 8).
This patch just disables initialization order fiasco detection for LTO 
for now. Tested and bootstrapped on x86_64-unknown-linux-gnu, OK to apply?
Or do I need to cook a proper fix for GCC 7 (and branches) and come back 
then?

-Maxim

[-- Attachment #2: pr79061-3.diff --]
[-- Type: text/x-diff, Size: 2289 bytes --]

gcc/ChangeLog:

2017-01-30  Maxim Ostapenko  <m.ostapenko@samsung.com>

	* asan.c (get_translation_unit_decl): Remove function.
	(asan_add_global): Force has_dynamic_init to zero in LTO mode.

diff --git a/gcc/asan.c b/gcc/asan.c
index 9098121..6cdd59b 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -2373,22 +2373,6 @@ asan_needs_odr_indicator_p (tree decl)
 	  && TREE_PUBLIC (decl));
 }
 
-/* For given DECL return its corresponding TRANSLATION_UNIT_DECL.  */
-
-static const_tree
-get_translation_unit_decl (tree decl)
-{
-  const_tree context = decl;
-  while (context && TREE_CODE (context) != TRANSLATION_UNIT_DECL)
-    {
-      if (TREE_CODE (context) == BLOCK)
-	context = BLOCK_SUPERCONTEXT (context);
-      else
-	context = get_containing_scope (context);
-    }
-  return context;
-}
-
 /* Append description of a single global DECL into vector V.
    TYPE is __asan_global struct type as returned by asan_global_struct.  */
 
@@ -2408,14 +2392,7 @@ asan_add_global (tree decl, tree type, vec<constructor_elt, va_gc> *v)
     pp_string (&asan_pp, "<unknown>");
   str_cst = asan_pp_string (&asan_pp);
 
-  const char *filename = main_input_filename;
-  if (in_lto_p)
-    {
-      const_tree translation_unit_decl = get_translation_unit_decl (decl);
-      if (translation_unit_decl && DECL_NAME (translation_unit_decl) != NULL)
-	filename = IDENTIFIER_POINTER (DECL_NAME (translation_unit_decl));
-    }
-  pp_string (&module_name_pp, filename);
+  pp_string (&module_name_pp, main_input_filename);
   module_name_cst = asan_pp_string (&module_name_pp);
 
   if (asan_needs_local_alias (decl))
@@ -2451,7 +2428,11 @@ asan_add_global (tree decl, tree type, vec<constructor_elt, va_gc> *v)
   CONSTRUCTOR_APPEND_ELT (vinner, NULL_TREE,
 			  fold_convert (const_ptr_type_node, module_name_cst));
   varpool_node *vnode = varpool_node::get (decl);
-  int has_dynamic_init = vnode ? vnode->dynamically_initialized : 0;
+  int has_dynamic_init = 0;
+  /* FIXME: Enable initialization order fiasco detection in LTO mode once
+     proper fix for PR 79061 will be applied.  */
+  if (!in_lto_p)
+    has_dynamic_init = vnode ? vnode->dynamically_initialized : 0;
   CONSTRUCTOR_APPEND_ELT (vinner, NULL_TREE,
 			  build_int_cst (uptr, has_dynamic_init));
   tree locptr = NULL_TREE;

Re: [PATCH v3][PR lto/79061] Fix LTO plus ASAN fails with "AddressSanitizer: initialization-order-fiasco".

[Richard Biener]

On Mon, 30 Jan 2017, Maxim Ostapenko wrote:

> Hi,
> 
> as was figured out in PR, using DECL_NAME (TRANSLATION_UNIT_DECL) does not
> always give us a correct module name in LTO mode because e.g. DECL_CONTEXT of
> some variables can be NAMESPACE_DECL and LTO merges NAMESPACE_DECLs.

Yes, it indeed does.  Note that GCC8+ both TU decls and NAMESPACE_DECLs
should no longer be neccessary and eventually vanish completely...
(in lto1, that is).  Can we code-gen the init order stuff early before
LTO write-out?

> The easiest fix is just to disable the initialization order checking altogether
> for LTO by forcing dynamically_initialized = 0 in LTO for now but we'll
> probably want to restore initialization order fiasco detection in the future
> (e.g. for GCC 8).
> This patch just disables initialization order fiasco detection for LTO for
> now. Tested and bootstrapped on x86_64-unknown-linux-gnu, OK to apply?
> Or do I need to cook a proper fix for GCC 7 (and branches) and come back then?

The patch works for me.

Richard.

Re: [PATCH v3][PR lto/79061] Fix LTO plus ASAN fails with "AddressSanitizer: initialization-order-fiasco".

[Jakub Jelinek]

On Mon, Jan 30, 2017 at 06:09:36PM +0300, Maxim Ostapenko wrote:
> Hi,
> 
> as was figured out in PR, using DECL_NAME (TRANSLATION_UNIT_DECL) does not
> always give us a correct module name in LTO mode because e.g. DECL_CONTEXT
> of some variables can be NAMESPACE_DECL and LTO merges NAMESPACE_DECLs. The
> easiest fix is just to disable the initialization order checking altogether
> for LTO by forcing dynamically_initialized = 0 in LTO for now but we'll
> probably want to restore initialization order fiasco detection in the future
> (e.g. for GCC 8).
> This patch just disables initialization order fiasco detection for LTO for
> now. Tested and bootstrapped on x86_64-unknown-linux-gnu, OK to apply?
> Or do I need to cook a proper fix for GCC 7 (and branches) and come back
> then?
> 
> -Maxim

> gcc/ChangeLog:
> 
> 2017-01-30  Maxim Ostapenko  <m.ostapenko@samsung.com>
> 

Please add
	PR lto/79061
here.
> 	* asan.c (get_translation_unit_decl): Remove function.
> 	(asan_add_global): Force has_dynamic_init to zero in LTO mode.

Ok with that change.

	Jakub

Re: [PATCH v3][PR lto/79061] Fix LTO plus ASAN fails with "AddressSanitizer: initialization-order-fiasco".

[Jakub Jelinek]

On Mon, Jan 30, 2017 at 04:14:40PM +0100, Richard Biener wrote:
> > as was figured out in PR, using DECL_NAME (TRANSLATION_UNIT_DECL) does not
> > always give us a correct module name in LTO mode because e.g. DECL_CONTEXT of
> > some variables can be NAMESPACE_DECL and LTO merges NAMESPACE_DECLs.
> 
> Yes, it indeed does.  Note that GCC8+ both TU decls and NAMESPACE_DECLs
> should no longer be neccessary and eventually vanish completely...
> (in lto1, that is).  Can we code-gen the init order stuff early before
> LTO write-out?

The problem is that at least right now the handling of the init-order is
done in 2 separate places.
The C++ FE emits the special libasan calls with the name of the TU at the
beginning and end of the static initialization.
And then the variables need to be registered with the libasan runtime from
an even earlier constructor, and this is something that is done very late
(where we collect all the variables).
So the options for LTO are:
1) somewhere preserve (at least for dynamically_initialized vars) the TU
it came originally from, if some dynamically_initialized var is owned by
more TUs, just drop that flag
2) rewrite in LTO the dynamic_init libasan calls (register with the whole
LTO partition name rather than individual original TUs); this has the major
disadvantage that it will not diagnose initialization order bugs between
vars from TUs from the same LTO partition
3) create the table of global vars for dynamically_initialized vars early
(save the artificial array with the var addresses + ctor into LTO bytecode),
at least for LTO, and then just register the non-dynamically_initialized
vars later (not really good idea for non-LTO, we want to register all the
vars from the whole TU together

1) looks easiest to mebut can grow varpool_node struct by a pointer size

	Jakub

Re: [PATCH v3][PR lto/79061] Fix LTO plus ASAN fails with "AddressSanitizer: initialization-order-fiasco".

[Richard Biener]

On Mon, Jan 30, 2017 at 4:26 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Mon, Jan 30, 2017 at 04:14:40PM +0100, Richard Biener wrote:
>> > as was figured out in PR, using DECL_NAME (TRANSLATION_UNIT_DECL) does not
>> > always give us a correct module name in LTO mode because e.g. DECL_CONTEXT of
>> > some variables can be NAMESPACE_DECL and LTO merges NAMESPACE_DECLs.
>>
>> Yes, it indeed does.  Note that GCC8+ both TU decls and NAMESPACE_DECLs
>> should no longer be neccessary and eventually vanish completely...
>> (in lto1, that is).  Can we code-gen the init order stuff early before
>> LTO write-out?
>
> The problem is that at least right now the handling of the init-order is
> done in 2 separate places.
> The C++ FE emits the special libasan calls with the name of the TU at the
> beginning and end of the static initialization.
> And then the variables need to be registered with the libasan runtime from
> an even earlier constructor, and this is something that is done very late
> (where we collect all the variables).

So we are basically telling libasan a list of dynamic initialized vars plus
when they start/end being constructed so it can catch access to uninitialized
vars during init of others?

I don't see why we need to compose that list of vars so late then.  Just
generate it early, say, after the first swoop of unused var removal.  Use
weak references so later removed ones get NULL.  Or simply bite the
bullet of asan changing code gen (well, it does that anyway) and thus
"pin" all vars life at that point as used.

Sounds much easier to me than carrying this over LTO...

And I suppose the TU name is only used for diagnostics?  Otherwise
the symbol name (DECL_ASSEMBLER_NAME) of the symbol could
be used as in C++ globals need to follow ODR?

> So the options for LTO are:
> 1) somewhere preserve (at least for dynamically_initialized vars) the TU
> it came originally from, if some dynamically_initialized var is owned by
> more TUs, just drop that flag
> 2) rewrite in LTO the dynamic_init libasan calls (register with the whole
> LTO partition name rather than individual original TUs); this has the major
> disadvantage that it will not diagnose initialization order bugs between
> vars from TUs from the same LTO partition
> 3) create the table of global vars for dynamically_initialized vars early
> (save the artificial array with the var addresses + ctor into LTO bytecode),
> at least for LTO, and then just register the non-dynamically_initialized
> vars later (not really good idea for non-LTO, we want to register all the
> vars from the whole TU together
>
> 1) looks easiest to mebut can grow varpool_node struct by a pointer size
>
>         Jakub

Re: [PATCH v3][PR lto/79061] Fix LTO plus ASAN fails with "AddressSanitizer: initialization-order-fiasco".

[Jakub Jelinek]

On Tue, Jan 31, 2017 at 08:59:27AM +0100, Richard Biener wrote:
> > The problem is that at least right now the handling of the init-order is
> > done in 2 separate places.
> > The C++ FE emits the special libasan calls with the name of the TU at the
> > beginning and end of the static initialization.
> > And then the variables need to be registered with the libasan runtime from
> > an even earlier constructor, and this is something that is done very late
> > (where we collect all the variables).
> 
> So we are basically telling libasan a list of dynamic initialized vars plus
> when they start/end being constructed so it can catch access to uninitialized
> vars during init of others?

We are telling libasan a list of all sanitized variables (those where we
managed to insert the padding around them in the end) and in that list mark
variables with dynamic initialization.  So, we can't e.g. register vars
where we for whatever reason can't add the padding around them, and this is
something that is decided when the variables are finalized.
Thus, constructing the list early (as a VAR_DECL with initializer) means
we'd then need to be able to remove stuff from it etc.
It looks much easier to me to carry this list only in the LTO data
structures (basically remember which TU owns each var).

> 
> I don't see why we need to compose that list of vars so late then.  Just
> generate it early, say, after the first swoop of unused var removal.  Use
> weak references so later removed ones get NULL.  Or simply bite the
> bullet of asan changing code gen (well, it does that anyway) and thus
> "pin" all vars life at that point as used.
> 
> Sounds much easier to me than carrying this over LTO...
> 
> And I suppose the TU name is only used for diagnostics?  Otherwise
> the symbol name (DECL_ASSEMBLER_NAME) of the symbol could
> be used as in C++ globals need to follow ODR?

The way it works is that you register the sanitized variables and each var
has a string for the owning TU (it is used also for diagnostics, so it is
desirable to not use random strings).  Then when the start of dynamic
initialization for some TU is called, libasan poisons all dynamic_init
global variables except those that have the owning TU string equal to the
current TU.  Then the construction is run (which means it will fail
if it accesses a dynamically initialized variable from some other TU),
and finally another libasan routine is called which will unpoison all the
variables it poisoned earlier.  So, for this use it is desirable the
names are actually the TU names, what you pass in corresponding static
initialization to the libasan functions.

	Jakub

Re: [PATCH v3][PR lto/79061] Fix LTO plus ASAN fails with "AddressSanitizer: initialization-order-fiasco".

[Maxim Ostapenko]

[-- Attachment #1: Type: text/plain, Size: 112 bytes --]

Hi,

PR lto/79061 actually affects gcc-{5, 6}-branch too
Is it OK to apply following patch on branches?

-Maxim

[-- Attachment #2: pr79061.diff --]
[-- Type: text/x-diff, Size: 944 bytes --]

gcc/ChangeLog:

2017-02-02  Maxim Ostapenko  <m.ostapenko@samsung.com>

	PR lto/79061
	* asan.c (asan_add_global): Force has_dynamic_init to zero in LTO mode.

diff --git a/gcc/asan.c b/gcc/asan.c
index 398a508..0f55dc0 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -2275,7 +2275,11 @@ asan_add_global (tree decl, tree type, vec<constructor_elt, va_gc> *v)
   CONSTRUCTOR_APPEND_ELT (vinner, NULL_TREE,
 			  fold_convert (const_ptr_type_node, module_name_cst));
   varpool_node *vnode = varpool_node::get (decl);
-  int has_dynamic_init = vnode ? vnode->dynamically_initialized : 0;
+  int has_dynamic_init = 0;
+  /* FIXME: Enable initialization order fiasco detection in LTO mode once
+     proper fix for PR 79061 will be applied.  */
+  if (!in_lto_p)
+    has_dynamic_init = vnode ? vnode->dynamically_initialized : 0;
   CONSTRUCTOR_APPEND_ELT (vinner, NULL_TREE,
 			  build_int_cst (uptr, has_dynamic_init));
   tree locptr = NULL_TREE;

Re: [PATCH v3][PR lto/79061] Fix LTO plus ASAN fails with "AddressSanitizer: initialization-order-fiasco".

[Jakub Jelinek]

On Thu, Feb 02, 2017 at 06:02:07PM +0300, Maxim Ostapenko wrote:
> Hi,
> 
> PR lto/79061 actually affects gcc-{5, 6}-branch too
> Is it OK to apply following patch on branches?
> 
> -Maxim

> gcc/ChangeLog:
> 
> 2017-02-02  Maxim Ostapenko  <m.ostapenko@samsung.com>
> 
> 	PR lto/79061
> 	* asan.c (asan_add_global): Force has_dynamic_init to zero in LTO mode.

Ok, thanks.

	Jakub