Re: [gimplefe] Regarding command line option handling
On Wed, May 4, 2016 at 4:29 PM, Prasad Ghangal wrote:
> On 4 May 2016 at 15:54, Richard Biener wrote:
>> On Wed, May 4, 2016 at 11:46 AM, Prasad Ghangal
>> wrote:
>>> On 4 May 2016 at 13:02, Richard Biener wrote:
On Wed, May 4, 2016 at 8:41 AM, Prasad Ghangal
wrote:
> Hi !
> Currently I am trying to introduce new command line option -fgimple,
> for that I am adding this to c.opt
>
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index 4f86876..88e55c6 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -66,6 +66,10 @@ C ObjC C++ ObjC++ Separate Alias(d)
> -dump=
> C ObjC C++ ObjC++ Joined Alias(d)
>
> +fgimple
> +C Var(flag_gimple) Init(0)
> +Enable parsing GIMPLE
> +
> -imacros
> C ObjC C++ ObjC++ Separate Alias(imacros) MissingArgError(missing
> filename after %qs)
>
>
> But I am getting error as - "gcc: error: unrecognized command line
> option ‘-fgimple ’; did you mean ‘-fgimple ’?"
>
> I am unable to find where to handle it.
Did you properly re-build gcc after that change? It should work just fine.
Richard.
>>>
>>> Yes, I did stage 1 build on latest revision. Still it's giving this
>>> strange error - "gcc: error: unrecognized command line option
>>> ‘-fgimple’; did you mean ‘-fgimple’?"
>>
>> The error is indeed strage. W/o the patch I get
>>
>>> ./xgcc -B. -fgimple -S t.i
>> xgcc: error: unrecognized command line option '-fgimple'; did you mean
>> '--compile'?
>>
>> and with it (re-building cc1 and xgcc inside gcc/ of my dev-tree)
>>
>>> ./xgcc -B. -fgimple -S t.i
>> (no error)
>>
>> so it works fine for me. Note that 'gcc' inside the build tree is called
>> xgcc.
>>
>
> Yes, it works for $build/gcc/cc1 but not for $prefix/bin/gcc i.e.
> after installing
Does it work for $build/gcc/xgcc?
Richard.
>> Richard.
>>
>>
>>>
>
>
> Till now I am able to JUST parse __GIMPLE keyword
>
> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> index cae2faf..1ccb4d6 100644
> --- a/gcc/c-family/c-common.c
> +++ b/gcc/c-family/c-common.c
> @@ -468,6 +468,7 @@ const struct c_common_resword c_common_reswords[] =
>{ "__extension__",RID_EXTENSION,0 },
>{ "__func__",RID_C99_FUNCTION_NAME, 0 },
>{ "__has_nothrow_assign", RID_HAS_NOTHROW_ASSIGN, D_CXXONLY },
> + { "__GIMPLE",RID_GIMPLE,0 },
>{ "__has_nothrow_constructor", RID_HAS_NOTHROW_CONSTRUCTOR, D_CXXONLY
> },
>{ "__has_nothrow_copy", RID_HAS_NOTHROW_COPY, D_CXXONLY },
>{ "__has_trivial_assign", RID_HAS_TRIVIAL_ASSIGN, D_CXXONLY },
> @@ -12393,6 +12394,7 @@ keyword_is_function_specifier (enum rid keyword)
> case RID_NORETURN:
> case RID_VIRTUAL:
> case RID_EXPLICIT:
> +case RID_GIMPLE:
>return true;
> default:
>return false;
> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> index 663e457..a91665f 100644
> --- a/gcc/c-family/c-common.h
> +++ b/gcc/c-family/c-common.h
> @@ -64,10 +64,10 @@ enum rid
>/* Modifiers: */
>/* C, in empirical order of frequency. */
>RID_STATIC = 0,
> - RID_UNSIGNED, RID_LONG,RID_CONST, RID_EXTERN,
> - RID_REGISTER, RID_TYPEDEF, RID_SHORT, RID_INLINE,
> - RID_VOLATILE, RID_SIGNED, RID_AUTO, RID_RESTRICT,
> - RID_NORETURN, RID_ATOMIC,
> + RID_UNSIGNED, RID_LONG, RID_CONST, RID_EXTERN,
> + RID_GIMPLE,RID_REGISTER, RID_TYPEDEF, RID_SHORT,
> + RID_INLINE,RID_VOLATILE, RID_SIGNED, RID_AUTO,
> + RID_RESTRICT, RID_NORETURN, RID_ATOMIC,
>
>/* C extensions */
>RID_COMPLEX, RID_THREAD, RID_SAT,
> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
> index f0c677b..e690ca3 100644
> --- a/gcc/c/c-decl.c
> +++ b/gcc/c/c-decl.c
> @@ -10401,6 +10401,8 @@ declspecs_add_scspec (source_location loc,
> case RID_TYPEDEF:
>n = csc_typedef;
>break;
> +case RID_GIMPLE:
> + break;
> default:
>gcc_unreachable ();
> }
> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
> index bdd669d..266b672 100644
> --- a/gcc/c/c-parser.c
> +++ b/gcc/c/c-parser.c
> @@ -732,6 +732,7 @@ c_token_starts_declspecs (c_token *token)
> case RID_ALIGNAS:
> case RID_ATOMIC:
> case RID_AUTO_TYPE:
> +case RID_GIMPLE:
>return true;
> default:
>if (token->keyword >= RID_FIRST_INT_N
> @@ -2461,6 +2462,7 @@ c_parser_declspecs (c_parser *parser, struct
> c_declspecs *specs,
> case RID_NORETURN:
> case RID_AUTO:
> case RID_THREAD:
> +case RID_GIMPLE:
>if (!scspec_ok)
> goto out;
>attrs_ok = true;
> @@ -3960,6 +3962,7 @@ c_parser_
Re: increase alignment of global structs in increase_alignment pass
On Wed, 4 May 2016, Prathamesh Kulkarni wrote:
> On 23 February 2016 at 21:49, Prathamesh Kulkarni
> wrote:
> > On 23 February 2016 at 17:31, Richard Biener wrote:
> >> On Tue, 23 Feb 2016, Prathamesh Kulkarni wrote:
> >>
> >>> On 22 February 2016 at 17:36, Richard Biener wrote:
> >>> > On Mon, 22 Feb 2016, Prathamesh Kulkarni wrote:
> >>> >
> >>> >> Hi Richard,
> >>> >> As discussed in private mail, this version of patch attempts to
> >>> >> increase alignment
> >>> >> of global struct decl if it contains an an array field(s) and array's
> >>> >> offset is a multiple of the alignment of vector type corresponding to
> >>> >> it's scalar type and recursively checks for nested structs.
> >>> >> eg:
> >>> >> static struct
> >>> >> {
> >>> >> int a, b, c, d;
> >>> >> int k[4];
> >>> >> float f[10];
> >>> >> };
> >>> >> k is a candidate array since it's offset is 16 and alignment of
> >>> >> "vector (4) int" is 8.
> >>> >> Similarly for f.
> >>> >>
> >>> >> I haven't been able to create a test-case where there are
> >>> >> multiple candidate arrays and vector alignment of arrays are different.
> >>> >> I suppose in this case we will have to increase alignment
> >>> >> of the struct by the max alignment ?
> >>> >> eg:
> >>> >> static struct
> >>> >> {
> >>> >>
> >>> >> T1 k[S1]
> >>> >>
> >>> >> T2 f[S2]
> >>> >>
> >>> >> };
> >>> >>
> >>> >> if V1 is vector type corresponding to T1, and V2 corresponding vector
> >>> >> type to T2,
> >>> >> offset (k) % align(V1) == 0 and offset (f) % align(V2) == 0
> >>> >> and align (V1) > align(V2) then we will increase alignment of struct
> >>> >> by align(V1).
> >>> >>
> >>> >> Testing showed FAIL for g++.dg/torture/pr31863.C due to program
> >>> >> timeout.
> >>> >> Initially it appeared to me, it went in infinite loop. However
> >>> >> on second thoughts I think it's probably not an infinite loop, rather
> >>> >> taking (extraordinarily) large amount of time
> >>> >> to compile the test-case with the patch.
> >>> >> The test-case builds quickly for only 2 instantiations of ClassSpec
> >>> >> (ClassSpec,
> >>> >> ClassSpec)
> >>> >> Building with 22 instantiations (upto ClassSpec) takes
> >>> >> up
> >>> >> to ~1m to compile.
> >>> >> with:
> >>> >> 23 instantiations: ~2m
> >>> >> 24 instantiations: ~5m
> >>> >> For 30 instantiations I terminated cc1plus after 13m (by SIGKILL).
> >>> >>
> >>> >> I guess it shouldn't go in an infinite loop because:
> >>> >> a) structs cannot have circular references.
> >>> >> b) works for lower number of instantiations
> >>> >> However I have no sound evidence that it cannot be in infinite loop.
> >>> >> I don't understand why a decl node is getting visited more than once
> >>> >> for that test-case.
> >>> >>
> >>> >> Using a hash_map to store alignments of decl's so that decl node gets
> >>> >> visited
> >>> >> only once prevents the issue.
> >>> >
> >>> > Maybe aliases. Try not walking vnode->alias == true vars.
> >>> Hi,
> >>> I have a hypothesis why decl node gets visited multiple times.
> >>>
> >>> Consider the test-case:
> >>> template
> >>> struct X
> >>> {
> >>> T a;
> >>> virtual int foo() { return N; }
> >>> };
> >>>
> >>> typedef struct X x_1;
> >>> typedef struct X x_2;
> >>>
> >>> static x_1 obj1 __attribute__((used));
> >>> static x_2 obj2 __attribute__((used));
> >>>
> >>> Two additional structs are created by C++FE, c++filt shows:
> >>> _ZTI1XIiLj1EE -> typeinfo for X
> >>> _ZTI1XIiLj2EE -> typeinfo for X
> >>>
> >>> Both of these structs have only one field D.2991 and it appears it's
> >>> *shared* between them:
> >>> struct D.2991;
> >>> const void * D.2980;
> >>> const char * D.2981;
> >>>
> >>> Hence the decl node D.2991 and it's fields (D.2890, D.2981) get visited
> >>> twice:
> >>> once when walking _ZTI1XIiLj1EE and 2nd time when walking _ZTI1XIiLj2EE
> >>>
> >>> Dump of walking over the global structs for above test-case:
> >>> http://pastebin.com/R5SABW0c
> >>>
> >>> So it appears to me to me a DAG (interior node == struct decl, leaf ==
> >>> non-struct field,
> >>> edge from node1 -> node2 if node2 is field of node1) is getting
> >>> created when struct decl
> >>> is a type-info object.
> >>>
> >>> I am not really clear on how we should proceed:
> >>> a) Keep using hash_map to store alignments so that every decl gets
> >>> visited only once.
> >>> b) Skip walking artificial record decls.
> >>> I am not sure if skipping all artificial struct decls would be a good
> >>> idea, but I don't
> >>> think it's possible to identify if a struct-decl is typeinfo struct at
> >>> middle-end ?
> >>
> >> You shouldn't end up walking those when walking the type of
> >> global decls. That is, don't walk typeinfo decls - yes, practically
> >> that means just not walking DECL_ARTIFICIAL things.
> > Hi,
> > I have done the changes in the patch (attached) and cross-tested
> > on arm*-*-* and aarch64*-*-* without failures.
> > Is it OK for stage-1 ?
> Hi,
> Is the attached patch OK for trun
Re: Please, take '-Wmisleading-indentation' out of -Wall
On 05/05/2016 07:56 PM, Antonio Diaz Diaz wrote:
> Take this example http://gcc.gnu.org/ml/gcc-patches/2016-03/msg00261.html
>
> The user sees this:
>
>if (flagA) // GUARD
> foo (0); // BODY
> #if SOME_CONDITION_THAT_DOES_NOT_HOLD
>if (flagB)
> #endif
> foo (1); // NEXT
Surely this misleading code is exactly what we should be warning about.
It could be like this, and far less misleadingly:
if (flagA) // GUARD
foo (0); // BODY
#if SOME_CONDITION_THAT_DOES_NOT_HOLD
if (flagB)
#endif
{
foo (1); // NEXT
}
Better for the reader, nothing to warn about.
Andrew.
Re: [gimplefe] Regarding command line option handling
On 6 May 2016 at 16:09, Richard Biener wrote:
> On Wed, May 4, 2016 at 4:29 PM, Prasad Ghangal
> wrote:
>> On 4 May 2016 at 15:54, Richard Biener wrote:
>>> On Wed, May 4, 2016 at 11:46 AM, Prasad Ghangal
>>> wrote:
On 4 May 2016 at 13:02, Richard Biener wrote:
> On Wed, May 4, 2016 at 8:41 AM, Prasad Ghangal
> wrote:
>> Hi !
>> Currently I am trying to introduce new command line option -fgimple,
>> for that I am adding this to c.opt
>>
>> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
>> index 4f86876..88e55c6 100644
>> --- a/gcc/c-family/c.opt
>> +++ b/gcc/c-family/c.opt
>> @@ -66,6 +66,10 @@ C ObjC C++ ObjC++ Separate Alias(d)
>> -dump=
>> C ObjC C++ ObjC++ Joined Alias(d)
>>
>> +fgimple
>> +C Var(flag_gimple) Init(0)
>> +Enable parsing GIMPLE
>> +
>> -imacros
>> C ObjC C++ ObjC++ Separate Alias(imacros) MissingArgError(missing
>> filename after %qs)
>>
>>
>> But I am getting error as - "gcc: error: unrecognized command line
>> option ‘-fgimple ’; did you mean ‘-fgimple ’?"
>>
>> I am unable to find where to handle it.
>
> Did you properly re-build gcc after that change? It should work just
> fine.
>
> Richard.
>
Yes, I did stage 1 build on latest revision. Still it's giving this
strange error - "gcc: error: unrecognized command line option
‘-fgimple’; did you mean ‘-fgimple’?"
>>>
>>> The error is indeed strage. W/o the patch I get
>>>
./xgcc -B. -fgimple -S t.i
>>> xgcc: error: unrecognized command line option '-fgimple'; did you mean
>>> '--compile'?
>>>
>>> and with it (re-building cc1 and xgcc inside gcc/ of my dev-tree)
>>>
./xgcc -B. -fgimple -S t.i
>>> (no error)
>>>
>>> so it works fine for me. Note that 'gcc' inside the build tree is called
>>> xgcc.
>>>
>>
>> Yes, it works for $build/gcc/cc1 but not for $prefix/bin/gcc i.e.
>> after installing
>
> Does it work for $build/gcc/xgcc?
>
No. It doesn't work for me. I even tried bootstrapped build-install,
still same issue.
> Richard.
>
>>> Richard.
>>>
>>>
>>
>>
>> Till now I am able to JUST parse __GIMPLE keyword
>>
>> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
>> index cae2faf..1ccb4d6 100644
>> --- a/gcc/c-family/c-common.c
>> +++ b/gcc/c-family/c-common.c
>> @@ -468,6 +468,7 @@ const struct c_common_resword c_common_reswords[] =
>>{ "__extension__",RID_EXTENSION,0 },
>>{ "__func__",RID_C99_FUNCTION_NAME, 0 },
>>{ "__has_nothrow_assign", RID_HAS_NOTHROW_ASSIGN, D_CXXONLY },
>> + { "__GIMPLE",RID_GIMPLE,0 },
>>{ "__has_nothrow_constructor", RID_HAS_NOTHROW_CONSTRUCTOR, D_CXXONLY
>> },
>>{ "__has_nothrow_copy", RID_HAS_NOTHROW_COPY, D_CXXONLY },
>>{ "__has_trivial_assign", RID_HAS_TRIVIAL_ASSIGN, D_CXXONLY },
>> @@ -12393,6 +12394,7 @@ keyword_is_function_specifier (enum rid keyword)
>> case RID_NORETURN:
>> case RID_VIRTUAL:
>> case RID_EXPLICIT:
>> +case RID_GIMPLE:
>>return true;
>> default:
>>return false;
>> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
>> index 663e457..a91665f 100644
>> --- a/gcc/c-family/c-common.h
>> +++ b/gcc/c-family/c-common.h
>> @@ -64,10 +64,10 @@ enum rid
>>/* Modifiers: */
>>/* C, in empirical order of frequency. */
>>RID_STATIC = 0,
>> - RID_UNSIGNED, RID_LONG,RID_CONST, RID_EXTERN,
>> - RID_REGISTER, RID_TYPEDEF, RID_SHORT, RID_INLINE,
>> - RID_VOLATILE, RID_SIGNED, RID_AUTO, RID_RESTRICT,
>> - RID_NORETURN, RID_ATOMIC,
>> + RID_UNSIGNED, RID_LONG, RID_CONST, RID_EXTERN,
>> + RID_GIMPLE,RID_REGISTER, RID_TYPEDEF, RID_SHORT,
>> + RID_INLINE,RID_VOLATILE, RID_SIGNED, RID_AUTO,
>> + RID_RESTRICT, RID_NORETURN, RID_ATOMIC,
>>
>>/* C extensions */
>>RID_COMPLEX, RID_THREAD, RID_SAT,
>> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
>> index f0c677b..e690ca3 100644
>> --- a/gcc/c/c-decl.c
>> +++ b/gcc/c/c-decl.c
>> @@ -10401,6 +10401,8 @@ declspecs_add_scspec (source_location loc,
>> case RID_TYPEDEF:
>>n = csc_typedef;
>>break;
>> +case RID_GIMPLE:
>> + break;
>> default:
>>gcc_unreachable ();
>> }
>> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
>> index bdd669d..266b672 100644
>> --- a/gcc/c/c-parser.c
>> +++ b/gcc/c/c-parser.c
>> @@ -732,6 +732,7 @@ c_token_starts_declspecs (c_token *token)
>> case RID_ALIGNAS:
>> case RID_ATOMIC:
>> case RID_AUTO_TYPE:
>> +case RID_GIMPLE:
>>return true;
>> default:
>>if (token->keyword >= RID_FIRST_INT_N
>> @@ -2461,6 +2462,7 @@ c_parser_declspec
Re: SafeStack proposal in GCC
On Wed, Apr 20, 2016 at 06:09:54PM +0200, Volodymyr Kuznetsov wrote: > On Wed, Apr 20, 2016 at 4:54 PM, Szabolcs Nagy wrote: > > On 13/04/16 14:01, Cristina Georgiana Opriceana wrote: > >> I bring to your attention SafeStack, part of a bigger research project > >> - CPI/CPS [1], which offers complete protection against stack-based > >> control flow hijacks. > > > > i think it does not provide complete protection. > > > > it cannot instrument the c runtime or dsos and attacks > > can be retried on a forking server which has fixed memory > > layout, so there is still significant attack surface. > > > > (it would be nice if security experts made such claims > > much more carefully). > > OK, the point is well taken, every actual implementation of such > techniques is bound to have limitations like the ones you mention. The > protection can only be complete in an academic sense: assuming the > perfect implementation that follows the CPI formal model, has perfect > safe region isolation, etc. In practice, we're just trying to make the > attacker's job harder. > > > > >> In GCC, we propose a design composed of an instrumentation module > >> (implemented as a GIMPLE pass) and a runtime library. > > ... > >> The runtime support will have to deal with unsafe stack allocation - a > >> hook in the pthread create/destroy functions to create per-thread > >> stack regions. This runtime support might be reused from the Clang > >> implementation. > > > > the SafeStack runtime in compiler-rt has various issues > > that should be clearly documented. > > Thanks a lot for pointing it out, getting such feedback is one of the > reasons we want to upstream this in the first place. The other reasons > include getting more people to use and to improve the code by fixing > such issues. > > > > > it seems the runtime > > > > * aborts the process on allocation failure. > > > > * deallocates the unsafe stack using tsd dtors, but > > signal handlers may run between dtors and the actual > > thread exit.. without a mapped unsafe stack. > > I think the only correct way to handle this is to put the unsafe stack > allocation/deallocation code into libc. We implemented it for FreeBSD > libc in a way that imposes no overhead for apps that don't use > safestack, perhaps something similar could be possible in glibc. We'd > welcome any better suggestions on this! There's a trivial solution that has no allocation/deallocation complexities: __thread char __unsafestack[LARGEVALUE]; However I think a better alternative might be to use the system (for main thread) or libc-provided (for other threads) stack as the unsafe stack, and move the main stack pointer to point at a secondary safe stack, which could then be: __thread char __safestack[MEDIUMVALUE]; Since the unsafe stack is where large buffers can live, and the safe stack is basically just register spills/return addresses, the safe stack could reasonably be a fairly small, fixed size. > > * determines the main stack with broken heuristic > > (since the rlimit can change at runtime i don't think > > this is possible to do correctly in general). > > > > * interposes pthread_create but not c11 thrd_create > > so conforming c11 code will crash. (same for non-standard > > usage of raw clone.) > > > > * sigaltstack and swapcontext are broken too. > > We have prototype that supports swapcontext that we're happy to > release, but it clearly requires more work before being ready to merge > upstream. The *context APIs are deprecated and I'm not sure they're worth supporting with this. It would be a good excuse to get people to stop using them. > > > problems than the compiler parts: it has to be reliable > > and abi stable since safestack is advertised for > > production use. > > > > (i think gcc should raise the bar for runtime code > > quality higher than that, but there is precedent Agreed. > > for much worse runtimes in gcc so this should not > > block the safestack porting work, however consider > > these issues when communicating about it to upstream > > or to potential users.) Also agree. Rich
