diff mbox

[Pointer,Bounds,Checker,Builtins,instrumentation,3/5] Expand instrumented builtin calls

Message ID CAMbmDYZx-bMt6q3BMyNjrqqqPN15irPbGVE2oA6tu6rYGSZUWA@mail.gmail.com
State New
Headers show

Commit Message

Ilya Enkovich Nov. 18, 2014, 1:33 p.m. UTC
2014-11-18 15:18 GMT+03:00 Richard Biener <richard.guenther@gmail.com>:
> On Tue, Nov 18, 2014 at 1:13 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> 2014-11-18 15:04 GMT+03:00 Richard Biener <richard.guenther@gmail.com>:
>>> On Tue, Nov 18, 2014 at 11:51 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>> 2014-11-18 5:56 GMT+03:00 Jeff Law <law@redhat.com>:
>>>>> On 11/17/14 13:43, Ilya Enkovich wrote:
>>>>>
>>>>>>
>>>>>> I don't fully understand how this problem appears.  Is it fully AIX
>>>>>> specific and doesn't affect any other target?  May we put all _CHKP
>>>>>> codes to the end of enum and ignore them by AIX? Limiting number of
>>>>>> codes in enum due to restrictions of specific debug info format seems
>>>>>> weird.  If something cannot be encoded for debug info then it should
>>>>>> be ignored.  I really hoped that creation of functions by demand would
>>>>>> allow to avoid such kind of problems :(
>>>>>>
>>>>>> I'll try to make a patch reducing amound of builtin codes to a
>>>>>> required minimum in case it appears to be the best option we have.
>>>>>
>>>>> It's a problem with the AIX native tools.  Presumably they don't like really
>>>>> long lines -- it was a relatively common issue in the past with vendor
>>>>> tools.
>>>>>
>>>>> I think we should proceed on two fronts.  First if David could investigate
>>>>> the stabs-in-xcoff bits a bit to see if DBX_CONTIN_LEN can be used to
>>>>> resolve the issue.  Second if you could look at now duplicating every
>>>>> builtin in the enumeration since it's a combination of the number of enums
>>>>> and the length of the debug strings to represent them that's causing AIX
>>>>> heartburn.
>>>>>
>>>>>
>>>>>
>>>>> jeff
>>>>
>>>> I see.  I can reduce the number of _CHKP builtin codes. Experimental
>>>> patch shows resulting END_BUILTINS = 1242.  But we should expect
>>>> similar problem for i386 target builds hosted on AIX (here
>>>> http://toolchain.lug-owl.de/buildbot/ I see such build is tested).
>>>> Current IX86_BUILTIN_MAX is 2467 which is greater than current
>>>> END_BUILTINS.
>>>
>>> I think it's better to fix dbxout.c to do something sensible with
>>> too long lines for enum types.  It should be easy to cook up a
>>> testcase that is not GCC.
>>>
>>> enum { entry1, entry2, entry3.... };
>>>
>>
>> As I understand the main problem is that fixing dbxout in trunk won't
>> help to build stage1 compiler.
>
> Simply build stage1 without debug info on AIX then.
>
> Btw, you have to start fixing the bug at some point ... (we can
> backport to 4.8 and 4.9).  Of course dbxout.c is in kind of
> deep-freeze unmaintained state.

Seems with no continuation lines support there is no easy way to limit
stabstring length.  But it should be easy to fix this particular
problem (too long stabstring due to many enum values) by discarding
the rest of values when some string length threshold is hit.  Possible
patch:


With this we should be able to set limit in target config files.
Tried on linux with a small test with limit set to 20:

>cat test.c
enum e1 {
  val1, val2, val3
};
>gcc test.c -S -gstabs
>grep e1 test.s
        .stabs  "e1:T(0,21)=eval1:0,val2:1,;",128,0,0,0


The last enum value was discarded.

I don't know which target should set this limit and what limit values
should be used.

Thanks,
Ilya

>
> I don't think we ever encouraged work-arounds for broken host compilers
> if that requires substantial work.
>
>>> not sure how exactly the issue breaks AIX bootstrap though.  I suppose
>>> the assembler complains?
>>
>> It is linker how complaints. Here is a build log example:
>> http://toolchain.lug-owl.de/buildbot/deliver_artifact.php?mode=view&id=3073584
>
> I see.
>
> Thanks,
> Richard.
>
>> Ilya
>>
>>>
>>> Richard.
>>>
>>>> Ilya

Comments

David Edelsohn Nov. 18, 2014, 1:44 p.m. UTC | #1
On Tue, Nov 18, 2014 at 8:33 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2014-11-18 15:18 GMT+03:00 Richard Biener <richard.guenther@gmail.com>:
>> On Tue, Nov 18, 2014 at 1:13 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>> 2014-11-18 15:04 GMT+03:00 Richard Biener <richard.guenther@gmail.com>:
>>>> On Tue, Nov 18, 2014 at 11:51 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>> 2014-11-18 5:56 GMT+03:00 Jeff Law <law@redhat.com>:
>>>>>> On 11/17/14 13:43, Ilya Enkovich wrote:
>>>>>>
>>>>>>>
>>>>>>> I don't fully understand how this problem appears.  Is it fully AIX
>>>>>>> specific and doesn't affect any other target?  May we put all _CHKP
>>>>>>> codes to the end of enum and ignore them by AIX? Limiting number of
>>>>>>> codes in enum due to restrictions of specific debug info format seems
>>>>>>> weird.  If something cannot be encoded for debug info then it should
>>>>>>> be ignored.  I really hoped that creation of functions by demand would
>>>>>>> allow to avoid such kind of problems :(
>>>>>>>
>>>>>>> I'll try to make a patch reducing amound of builtin codes to a
>>>>>>> required minimum in case it appears to be the best option we have.
>>>>>>
>>>>>> It's a problem with the AIX native tools.  Presumably they don't like really
>>>>>> long lines -- it was a relatively common issue in the past with vendor
>>>>>> tools.
>>>>>>
>>>>>> I think we should proceed on two fronts.  First if David could investigate
>>>>>> the stabs-in-xcoff bits a bit to see if DBX_CONTIN_LEN can be used to
>>>>>> resolve the issue.  Second if you could look at now duplicating every
>>>>>> builtin in the enumeration since it's a combination of the number of enums
>>>>>> and the length of the debug strings to represent them that's causing AIX
>>>>>> heartburn.
>>>>>>
>>>>>>
>>>>>>
>>>>>> jeff
>>>>>
>>>>> I see.  I can reduce the number of _CHKP builtin codes. Experimental
>>>>> patch shows resulting END_BUILTINS = 1242.  But we should expect
>>>>> similar problem for i386 target builds hosted on AIX (here
>>>>> http://toolchain.lug-owl.de/buildbot/ I see such build is tested).
>>>>> Current IX86_BUILTIN_MAX is 2467 which is greater than current
>>>>> END_BUILTINS.
>>>>
>>>> I think it's better to fix dbxout.c to do something sensible with
>>>> too long lines for enum types.  It should be easy to cook up a
>>>> testcase that is not GCC.
>>>>
>>>> enum { entry1, entry2, entry3.... };
>>>>
>>>
>>> As I understand the main problem is that fixing dbxout in trunk won't
>>> help to build stage1 compiler.
>>
>> Simply build stage1 without debug info on AIX then.
>>
>> Btw, you have to start fixing the bug at some point ... (we can
>> backport to 4.8 and 4.9).  Of course dbxout.c is in kind of
>> deep-freeze unmaintained state.
>
> Seems with no continuation lines support there is no easy way to limit
> stabstring length.  But it should be easy to fix this particular
> problem (too long stabstring due to many enum values) by discarding
> the rest of values when some string length threshold is hit.  Possible
> patch:
>
> diff --git a/gcc/dbxout.c b/gcc/dbxout.c
> index aa15a39..9b0b82d 100644
> --- a/gcc/dbxout.c
> +++ b/gcc/dbxout.c
> @@ -168,6 +168,10 @@ along with GCC; see the file COPYING3.  If not see
>  #define DBX_CONTIN_CHAR '\\'
>  #endif
>
> +#ifndef DBX_MAX_LENGTH
> +#define DBX_MAX_LENGTH 0
> +#endif
> +
>  enum typestatus {TYPE_UNSEEN, TYPE_XREF, TYPE_DEFINED};
>
>  /* Structure recording information about a C data type.
> @@ -2270,6 +2274,12 @@ dbxout_type (tree type, int full)
>           stabstr_C (',');
>           if (TREE_CHAIN (tem) != 0)
>             CONTIN;
> +
> +         /* Prevent too long strings rejected by linker.  */
> +         if (DBX_CONTIN_LENGTH == 0
> +             && DBX_MAX_LENGTH != 0
> +             && obstack_object_size (&stabstr_ob) > DBX_MAX_LENGTH)
> +           break;
>         }
>
>        stabstr_C (';');
>
> With this we should be able to set limit in target config files.
> Tried on linux with a small test with limit set to 20:
>
>>cat test.c
> enum e1 {
>   val1, val2, val3
> };
>>gcc test.c -S -gstabs
>>grep e1 test.s
>         .stabs  "e1:T(0,21)=eval1:0,val2:1,;",128,0,0,0
>
>
> The last enum value was discarded.
>
> I don't know which target should set this limit and what limit values
> should be used.

Hi, Ilya

Thanks for the preliminary patch to dbxout.c.  I was planning to work
on something similar today.

Continuations would have been a nice solution, but, as I wrote
earlier, AIX assembler does not honor stabstring continuations.  It's
one line, period. As much as can fit on one line.  I believe that AIX
allows up to 64K.

My work-around was not meant as a solution.  I was trying to allow GCC
to build on AIX while we work on a solution and trying to help
everyone understand the source of the problem.

We definitely need a fix for dbxout in GCC and backport it to GCC 4.8
and GCC 4.9 branches.

I still think that it is rather unfortunate that the infrastructure
for CHKP introduces so much bloat into GCC on every target and every
architecture even when it is not enabled and cannot function.

Thanks, David
Richard Biener Nov. 18, 2014, 2:07 p.m. UTC | #2
On Tue, Nov 18, 2014 at 2:44 PM, David Edelsohn <dje.gcc@gmail.com> wrote:
> On Tue, Nov 18, 2014 at 8:33 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> 2014-11-18 15:18 GMT+03:00 Richard Biener <richard.guenther@gmail.com>:
>>> On Tue, Nov 18, 2014 at 1:13 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>> 2014-11-18 15:04 GMT+03:00 Richard Biener <richard.guenther@gmail.com>:
>>>>> On Tue, Nov 18, 2014 at 11:51 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>> 2014-11-18 5:56 GMT+03:00 Jeff Law <law@redhat.com>:
>>>>>>> On 11/17/14 13:43, Ilya Enkovich wrote:
>>>>>>>
>>>>>>>>
>>>>>>>> I don't fully understand how this problem appears.  Is it fully AIX
>>>>>>>> specific and doesn't affect any other target?  May we put all _CHKP
>>>>>>>> codes to the end of enum and ignore them by AIX? Limiting number of
>>>>>>>> codes in enum due to restrictions of specific debug info format seems
>>>>>>>> weird.  If something cannot be encoded for debug info then it should
>>>>>>>> be ignored.  I really hoped that creation of functions by demand would
>>>>>>>> allow to avoid such kind of problems :(
>>>>>>>>
>>>>>>>> I'll try to make a patch reducing amound of builtin codes to a
>>>>>>>> required minimum in case it appears to be the best option we have.
>>>>>>>
>>>>>>> It's a problem with the AIX native tools.  Presumably they don't like really
>>>>>>> long lines -- it was a relatively common issue in the past with vendor
>>>>>>> tools.
>>>>>>>
>>>>>>> I think we should proceed on two fronts.  First if David could investigate
>>>>>>> the stabs-in-xcoff bits a bit to see if DBX_CONTIN_LEN can be used to
>>>>>>> resolve the issue.  Second if you could look at now duplicating every
>>>>>>> builtin in the enumeration since it's a combination of the number of enums
>>>>>>> and the length of the debug strings to represent them that's causing AIX
>>>>>>> heartburn.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> jeff
>>>>>>
>>>>>> I see.  I can reduce the number of _CHKP builtin codes. Experimental
>>>>>> patch shows resulting END_BUILTINS = 1242.  But we should expect
>>>>>> similar problem for i386 target builds hosted on AIX (here
>>>>>> http://toolchain.lug-owl.de/buildbot/ I see such build is tested).
>>>>>> Current IX86_BUILTIN_MAX is 2467 which is greater than current
>>>>>> END_BUILTINS.
>>>>>
>>>>> I think it's better to fix dbxout.c to do something sensible with
>>>>> too long lines for enum types.  It should be easy to cook up a
>>>>> testcase that is not GCC.
>>>>>
>>>>> enum { entry1, entry2, entry3.... };
>>>>>
>>>>
>>>> As I understand the main problem is that fixing dbxout in trunk won't
>>>> help to build stage1 compiler.
>>>
>>> Simply build stage1 without debug info on AIX then.
>>>
>>> Btw, you have to start fixing the bug at some point ... (we can
>>> backport to 4.8 and 4.9).  Of course dbxout.c is in kind of
>>> deep-freeze unmaintained state.
>>
>> Seems with no continuation lines support there is no easy way to limit
>> stabstring length.  But it should be easy to fix this particular
>> problem (too long stabstring due to many enum values) by discarding
>> the rest of values when some string length threshold is hit.  Possible
>> patch:
>>
>> diff --git a/gcc/dbxout.c b/gcc/dbxout.c
>> index aa15a39..9b0b82d 100644
>> --- a/gcc/dbxout.c
>> +++ b/gcc/dbxout.c
>> @@ -168,6 +168,10 @@ along with GCC; see the file COPYING3.  If not see
>>  #define DBX_CONTIN_CHAR '\\'
>>  #endif
>>
>> +#ifndef DBX_MAX_LENGTH
>> +#define DBX_MAX_LENGTH 0
>> +#endif
>> +
>>  enum typestatus {TYPE_UNSEEN, TYPE_XREF, TYPE_DEFINED};
>>
>>  /* Structure recording information about a C data type.
>> @@ -2270,6 +2274,12 @@ dbxout_type (tree type, int full)
>>           stabstr_C (',');
>>           if (TREE_CHAIN (tem) != 0)
>>             CONTIN;
>> +
>> +         /* Prevent too long strings rejected by linker.  */
>> +         if (DBX_CONTIN_LENGTH == 0
>> +             && DBX_MAX_LENGTH != 0
>> +             && obstack_object_size (&stabstr_ob) > DBX_MAX_LENGTH)
>> +           break;
>>         }
>>
>>        stabstr_C (';');
>>
>> With this we should be able to set limit in target config files.
>> Tried on linux with a small test with limit set to 20:
>>
>>>cat test.c
>> enum e1 {
>>   val1, val2, val3
>> };
>>>gcc test.c -S -gstabs
>>>grep e1 test.s
>>         .stabs  "e1:T(0,21)=eval1:0,val2:1,;",128,0,0,0
>>
>>
>> The last enum value was discarded.
>>
>> I don't know which target should set this limit and what limit values
>> should be used.
>
> Hi, Ilya
>
> Thanks for the preliminary patch to dbxout.c.  I was planning to work
> on something similar today.
>
> Continuations would have been a nice solution, but, as I wrote
> earlier, AIX assembler does not honor stabstring continuations.

Can we emit this particular STABS piece in "raw" form then?  Thus
does it work to have sth like

   .stabs ...
   .string "..."
   .stabs ...

or whatever way to emit assembled data there?

>  It's
> one line, period. As much as can fit on one line.  I believe that AIX
> allows up to 64K.

Can we get the assembler fixed? (sorry to ask this stupid question
again ...)

> My work-around was not meant as a solution.  I was trying to allow GCC
> to build on AIX while we work on a solution and trying to help
> everyone understand the source of the problem.
>
> We definitely need a fix for dbxout in GCC and backport it to GCC 4.8
> and GCC 4.9 branches.
>
> I still think that it is rather unfortunate that the infrastructure
> for CHKP introduces so much bloat into GCC on every target and every
> architecture even when it is not enabled and cannot function.

True, but as said if you try to build a cross to x86_64 on AIX you'd
have hit the issue much earlier.

Richard.

> Thanks, David
David Edelsohn Nov. 18, 2014, 2:32 p.m. UTC | #3
On Tue, Nov 18, 2014 at 9:07 AM, Richard Biener
<richard.guenther@gmail.com> wrote:

> Can we emit this particular STABS piece in "raw" form then?  Thus
> does it work to have sth like
>
>    .stabs ...
>    .string "..."
>    .stabs ...
>
> or whatever way to emit assembled data there?

The stabstring debug information is saved as a length and a string.
For 32 bit AIX, the length is an unsigned short.  Longer entries
apparently can be stored in multiple symbol table entries, but it's
not clear how to emit assembly to produce that effect.  For 64 bit
AIX, the length is an unsigned int 32 bits, so one may be able to work
around this problem by compiling GCC on AIX in 64 bit mode.  A
conversion to default 64 bit build will take a lot of work.

Thanks, David
diff mbox

Patch

diff --git a/gcc/dbxout.c b/gcc/dbxout.c
index aa15a39..9b0b82d 100644
--- a/gcc/dbxout.c
+++ b/gcc/dbxout.c
@@ -168,6 +168,10 @@  along with GCC; see the file COPYING3.  If not see
 #define DBX_CONTIN_CHAR '\\'
 #endif

+#ifndef DBX_MAX_LENGTH
+#define DBX_MAX_LENGTH 0
+#endif
+
 enum typestatus {TYPE_UNSEEN, TYPE_XREF, TYPE_DEFINED};

 /* Structure recording information about a C data type.
@@ -2270,6 +2274,12 @@  dbxout_type (tree type, int full)
          stabstr_C (',');
          if (TREE_CHAIN (tem) != 0)
            CONTIN;
+
+         /* Prevent too long strings rejected by linker.  */
+         if (DBX_CONTIN_LENGTH == 0
+             && DBX_MAX_LENGTH != 0
+             && obstack_object_size (&stabstr_ob) > DBX_MAX_LENGTH)
+           break;
        }

       stabstr_C (';');