Bug 45362 - [4.6 Regression] Dangling reference about saved cpp_macro for push/pop macro
Summary: [4.6 Regression] Dangling reference about saved cpp_macro for push/pop macro
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: preprocessor (show other bugs)
Version: 4.6.0
: P3 normal
Target Milestone: 4.6.0
Assignee: Kai Tietz
URL:
Keywords: build, GC, ice-on-valid-code
: 45666 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-08-20 18:53 UTC by Kai Tietz
Modified: 2010-09-29 18:33 UTC (History)
5 users (show)

See Also:
Host:
Target: *-*-*
Build:
Known to work:
Known to fail:
Last reconfirmed: 2010-09-17 18:37:19


Attachments
testcase for problem (31.52 KB, application/octet-stream)
2010-09-17 14:01 UTC, Kai Tietz
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kai Tietz 2010-08-20 18:53:43 UTC
The issue is that for the push/pop macro the old state of the macro (a cpp_macro reference) is stored. As this structure is handled by GC without a root, all get free'ed when garbage collection happens.
This gc can lead to issues when such a saved node gets undefined and the node, which previously hold the cpp_macro reference, gets reused for a different macro. As the linked in the saved macro list isn't under control of gc and it doesn't have a gc root element, the stored reference gets invalid in such cases and can lead to segmentation faults due access to already free'ed memory.
Comment 1 Kai Tietz 2010-09-14 05:46:19 UTC
*** Bug 45666 has been marked as a duplicate of this bug. ***
Comment 2 Manuel López-Ibáñez 2010-09-14 09:32:08 UTC
http://gcc.gnu.org/bugs/#need
Comment 3 Andrew Pinski 2010-09-14 22:34:43 UTC
*** Bug 45666 has been marked as a duplicate of this bug. ***
Comment 4 Andrew Pinski 2010-09-14 22:35:53 UTC
(In reply to comment #2)
> http://gcc.gnu.org/bugs/#need

Since this is a bug in the preprocessor it is hard to get a preprocessed source that causes a bug.
Comment 5 t66667@gmail.com 2010-09-15 08:56:11 UTC
Peeled this skin (164193) off and then blood comes running out.
Comment 6 Kai Tietz 2010-09-16 16:56:53 UTC
(In reply to comment #4)
> (In reply to comment #2)
> > http://gcc.gnu.org/bugs/#need
> 
> Since this is a bug in the preprocessor it is hard to get a preprocessed source
> that causes a bug.
> 

That's true. Interesting is that by doing preprocessed source out of it, result is correct. Just within compiler it makes troubles, as here garbage collector gets raised, which cleans up with store reference, as a root element for preprocessor tokens is missing to keep it alive.
Comment 7 Manuel López-Ibáñez 2010-09-16 17:04:34 UTC
(In reply to comment #4)
> (In reply to comment #2)
> > http://gcc.gnu.org/bugs/#need
> 
> Since this is a bug in the preprocessor it is hard to get a preprocessed source
> that causes a bug.
> 

I think this is covered by:

<blockquote>The only excuses to not send us the preprocessed sources are (i) if you've found a bug in the preprocessor,</blockquote>

A testcase is always nice, even if not preprocessed.
Comment 8 t66667@gmail.com 2010-09-16 21:58:34 UTC
(In reply to comment #4)
> (In reply to comment #2)
> > http://gcc.gnu.org/bugs/#need
> 
> Since this is a bug in the preprocessor it is hard to get a preprocessed source
> that causes a bug.
> 

This is very odd, because I noticed, it could not produce this for example the 'just installed' gcc but the fault is only in xgcc.
This can only means that something might not be correct in tree? or in the build process.
However as it can show that this blocks the user 2 minutes from completing a GCC build.
So why fail at the last minute...
Comment 9 Andrew Pinski 2010-09-16 22:00:32 UTC
GC issues normally don't show at different times depending on the layout of memory and such.  Sometimes it depends on env variables being slightly different.
Comment 10 t66667@gmail.com 2010-09-16 22:03:09 UTC
Program received signal SIGSEGV, Segmentation fault.
gt_ggc_mx_cpp_macro (x_p=<value optimized out>) at gtype-desc.c:2078
2078                  ((*x).params[i0]) ? HT_IDENT_TO_GCC_IDENT (HT_NODE (((*x).params[i0]))) : NULL;
(gdb) bt
#0  gt_ggc_mx_cpp_macro (x_p=<value optimized out>) at gtype-desc.c:2078
#1  0x00000000004caee5 in gt_ggc_mx_lang_tree_node (x_p=<value optimized out>) at ./gt-c-decl.h:537
#2  0x00000000004cb0c3 in gt_ggc_mx_lang_tree_node (x_p=<value optimized out>) at ./gt-c-decl.h:370
#3  0x00000000004cbcc8 in gt_ggc_mx_c_binding (x_p=<value optimized out>) at ./gt-c-decl.h:103
#4  0x00000000004cbcf2 in gt_ggc_mx_c_binding (x_p=<value optimized out>) at ./gt-c-decl.h:106
#5  0x00000000004caea6 in gt_ggc_mx_lang_tree_node (x_p=<value optimized out>) at ./gt-c-decl.h:549
Comment 11 t66667@gmail.com 2010-09-16 22:06:49 UTC
But too bad this file 'gtype-desc.c' is automatically generated at build time.
Comment 12 Jakub Jelinek 2010-09-16 22:28:23 UTC
Can you try compiling it with
--param ggc-min-expand=0 --param ggc-min-heapsize=0
?  Perhaps you'll trigger it then more reliably...
Comment 13 t66667@gmail.com 2010-09-16 22:33:44 UTC
(In reply to comment #12)
> Can you try compiling it with
> --param ggc-min-expand=0 --param ggc-min-heapsize=0
> ?  Perhaps you'll trigger it then more reliably...
> 

Without it:
GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
Compiler executable checksum: 7ccff28a2de01cef50407f25bf137180

Program received signal SIGSEGV, Segmentation fault.
gt_ggc_mx_cpp_macro (x_p=<value optimized out>) at gtype-desc.c:2078
2078                  ((*x).params[i0]) ? HT_IDENT_TO_GCC_IDENT (HT_NODE (((*x).params[i0]))) : NULL;
(gdb) bt
#0  gt_ggc_mx_cpp_macro (x_p=<value optimized out>) at gtype-desc.c:2078

With it:
GGC heuristics: --param ggc-min-expand=0 --param ggc-min-heapsize=0
Compiler executable checksum: 7ccff28a2de01cef50407f25bf137180

Program received signal SIGSEGV, Segmentation fault.
gt_ggc_mx_cpp_macro (x_p=<value optimized out>) at gtype-desc.c:2078
2078                  ((*x).params[i0]) ? HT_IDENT_TO_GCC_IDENT (HT_NODE (((*x).params[i0]))) : NULL;
(gdb) bt
#0  gt_ggc_mx_cpp_macro (x_p=<value optimized out>) at gtype-desc.c:2078
#1  0x00000000004caee5 in gt_ggc_mx_lang_tree_node (x_p=<value optimized out>) at ./gt-c-decl.h:537
#2  0x00000000004cb0c3 in gt_ggc_mx_lang_tree_node (x_p=<value optimized out>) at ./gt-c-decl.h:370
#3  0x00000000004cbcc8 in gt_ggc_mx_c_binding (x_p=<value optimized out>) at ./gt-c-decl.h:103
#4  0x00000000004cbcf2 in gt_ggc_mx_c_binding (x_p=<value optimized out>) at ./gt-c-decl.h:106
#5  0x00000000004caea6 in gt_ggc_mx_lang_tree_node (x_p=<value optimized out>) at ./gt-c-decl.h:549
#6  0x00000000004cad62 in gt_ggc_mx_lang_tree_node (x_p=<value optimized out>) at ./gt-c-decl.h:350
#7  0x00000000004cb00d in gt_ggc_mx_lang_tree_node (x_p=<value optimized out>) at ./gt-c-decl.h:413
#8  0x00000000004caf8c in gt_ggc_mx_lang_tree_node (x_p=<value optimized out>) at ./gt-c-decl.h:393

I see no diff, do you mean compile the code that caused crash or compile the xgcc or gcc over again?
Comment 14 Kai Tietz 2010-09-17 14:01:18 UTC
Created attachment 21820 [details]
testcase for problem

As this test need more then on header, please extract it and compile then main.c to reproduce it. At least I was able to do this on linux64 by this testcase.
Comment 15 Kai Tietz 2010-09-17 18:37:19 UTC
(In reply to comment #14)
> Created an attachment (id=21820) [edit]
> testcase for problem
> 
> As this test need more then on header, please extract it and compile then
> main.c to reproduce it. At least I was able to do this on linux64 by this
> testcase.
> 

Patch for it posted to ML. See http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01394.html
Comment 16 t66667@gmail.com 2010-09-18 01:04:31 UTC
(In reply to comment #15)
> (In reply to comment #14)
> > Created an attachment (id=21820) [edit]
> > testcase for problem
> > 
> > As this test need more then on header, please extract it and compile then
> > main.c to reproduce it. At least I was able to do this on linux64 by this
> > testcase.
> > 
> 
> Patch for it posted to ML. See
> http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01394.html
> 

Hi, it failed produce problem with attached testcase both the gcc and xgcc.
I extracted it and compile with -c main.c
Comment 17 t66667@gmail.com 2010-09-18 01:14:22 UTC
(In reply to comment #15)
> (In reply to comment #14)
> > Created an attachment (id=21820) [edit]
> > testcase for problem
> > 
> > As this test need more then on header, please extract it and compile then
> > main.c to reproduce it. At least I was able to do this on linux64 by this
> > testcase.
> > 
> 
> Patch for it posted to ML. See
> http://gcc.gnu.org/ml/gcc-patches/2010-09/msg01394.html
> 

Thanks a-lot Kai! This patch fixes ICE in PR45666.
Comment 18 Kai Tietz 2010-09-29 18:18:42 UTC
Author: ktietz
Date: Wed Sep 29 18:18:38 2010
New Revision: 164729

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=164729
Log:
2010-09-29  Kai Tietz  <kai.tietz@onevision.com>

	PR preprocessor/45362
	* directives.c (cpp_pop_definition): Make static.
	(do_pragma_push_macro): Reworked to store text
	definition.
	(do_pragma_pop_macro): Add free text definition.
	(cpp_push_definition): Removed.
	* include/cpplib.h (cpp_push_definition): Removed.
	(cpp_pop_definition): Likewise.
	* internal.h (def_pragma_macro): Remove member 'value'
	and add new members 'definition', 'line',
	'syshdr', 'sued' and 'is_undef'.
	* pch.c (_cpp_restore_pushed_macros): Rework to work
	on text definition and store additional macro flags.
	(_cpp_save_pushed_macros): Likewise.


Modified:
    trunk/libcpp/ChangeLog
    trunk/libcpp/directives.c
    trunk/libcpp/include/cpplib.h
    trunk/libcpp/internal.h
    trunk/libcpp/pch.c
Comment 19 Kai Tietz 2010-09-29 18:33:50 UTC
Fixed on trunk.