diff mbox

[testsuite] PATCH: Add check_effective_target_pie

Message ID yddtwyssfm8.fsf@lokon.CeBiTec.Uni-Bielefeld.DE
State New
Headers show

Commit Message

Rainer Orth Feb. 11, 2015, 2:10 p.m. UTC
"H.J. Lu" <hjl.tools@gmail.com> writes:

>> The new proc is bogus, unfortunately: there's already an existing
>> check_effective_target_pie that checks if a target can support PIE.  The
>> new one just overrides the previous one.  On targets supporting PIE
>> (like Darwin), but not defaulting to it, the PIE tests suddenly turn out
>> UNSUPPORTED.
>>
>> You should rename the new one to
>> e.g. check_effective_target_pie_default, update the single user, and
>> document it in sourcebuild.texi.
>
> I checked in this as an obvious fix.

I think pie_enabled is not a very descriptive name:

With -fpie, PIE is also enabled, just not the default without any
options.  Please either go with the pie_default I sugested or wait for
others to weigh in before rushing in another `obvious' fix.

Thanks.
	Rainer

Comments

H.J. Lu Feb. 11, 2015, 2:20 p.m. UTC | #1
On Wed, Feb 11, 2015 at 6:10 AM, Rainer Orth
<ro@cebitec.uni-bielefeld.de> wrote:
> "H.J. Lu" <hjl.tools@gmail.com> writes:
>
>>> The new proc is bogus, unfortunately: there's already an existing
>>> check_effective_target_pie that checks if a target can support PIE.  The
>>> new one just overrides the previous one.  On targets supporting PIE
>>> (like Darwin), but not defaulting to it, the PIE tests suddenly turn out
>>> UNSUPPORTED.
>>>
>>> You should rename the new one to
>>> e.g. check_effective_target_pie_default, update the single user, and
>>> document it in sourcebuild.texi.
>>
>> I checked in this as an obvious fix.
>
> I think pie_enabled is not a very descriptive name:
>
> Index: doc/sourcebuild.texi
> ===================================================================
> --- doc/sourcebuild.texi (revision 220617)
> +++ doc/sourcebuild.texi (working copy)
> @@ -1884,6 +1884,9 @@
>  @item nonpic
>  Target does not generate PIC by default.
>
> +@item pie_enabled
> +Target generates PIE by default.
> +
>  @item pcc_bitfield_type_matters
>  Target defines @code{PCC_BITFIELD_TYPE_MATTERS}.
>
> With -fpie, PIE is also enabled, just not the default without any

I was testing

# make RUNTESTFLAGS="--target_board='unix{-m32\ -fpie,-fpie}'

I don't consider PIE is default.  It is just enabled.

> options.  Please either go with the pie_default I sugested or wait for
> others to weigh in before rushing in another `obvious' fix.
>
Rainer Orth Feb. 11, 2015, 3:19 p.m. UTC | #2
"H.J. Lu" <hjl.tools@gmail.com> writes:

> On Wed, Feb 11, 2015 at 6:10 AM, Rainer Orth
> <ro@cebitec.uni-bielefeld.de> wrote:
>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>>
>>>> The new proc is bogus, unfortunately: there's already an existing
>>>> check_effective_target_pie that checks if a target can support PIE.  The
>>>> new one just overrides the previous one.  On targets supporting PIE
>>>> (like Darwin), but not defaulting to it, the PIE tests suddenly turn out
>>>> UNSUPPORTED.
>>>>
>>>> You should rename the new one to
>>>> e.g. check_effective_target_pie_default, update the single user, and
>>>> document it in sourcebuild.texi.
>>>
>>> I checked in this as an obvious fix.
>>
>> I think pie_enabled is not a very descriptive name:
>>
>> Index: doc/sourcebuild.texi
>> ===================================================================
>> --- doc/sourcebuild.texi (revision 220617)
>> +++ doc/sourcebuild.texi (working copy)
>> @@ -1884,6 +1884,9 @@
>>  @item nonpic
>>  Target does not generate PIC by default.
>>
>> +@item pie_enabled
>> +Target generates PIE by default.
>> +
>>  @item pcc_bitfield_type_matters
>>  Target defines @code{PCC_BITFIELD_TYPE_MATTERS}.
>>
>> With -fpie, PIE is also enabled, just not the default without any
>
> I was testing
>
> # make RUNTESTFLAGS="--target_board='unix{-m32\ -fpie,-fpie}'
>
> I don't consider PIE is default.  It is just enabled.
>
>> options.  Please either go with the pie_default I sugested or wait for
>> others to weigh in before rushing in another `obvious' fix.

Then the description (both sourcebuild.texi and target-supports.texi) is
confusing.

What are you trying to achieve here, actually?  Even on Solaris 11/x86
(which doesn't support PIE), -fpie lets the
check_effective_target_pie_enabled (or whatever it's called) proc pass.
Shouldn't it also check if the target can support PIE at all?

Please clarify your goals before going forward with this.

	Rainer
H.J. Lu Feb. 11, 2015, 3:45 p.m. UTC | #3
On Wed, Feb 11, 2015 at 7:19 AM, Rainer Orth
<ro@cebitec.uni-bielefeld.de> wrote:
> "H.J. Lu" <hjl.tools@gmail.com> writes:
>
>> On Wed, Feb 11, 2015 at 6:10 AM, Rainer Orth
>> <ro@cebitec.uni-bielefeld.de> wrote:
>>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>>>
>>>>> The new proc is bogus, unfortunately: there's already an existing
>>>>> check_effective_target_pie that checks if a target can support PIE.  The
>>>>> new one just overrides the previous one.  On targets supporting PIE
>>>>> (like Darwin), but not defaulting to it, the PIE tests suddenly turn out
>>>>> UNSUPPORTED.
>>>>>
>>>>> You should rename the new one to
>>>>> e.g. check_effective_target_pie_default, update the single user, and
>>>>> document it in sourcebuild.texi.
>>>>
>>>> I checked in this as an obvious fix.
>>>
>>> I think pie_enabled is not a very descriptive name:
>>>
>>> Index: doc/sourcebuild.texi
>>> ===================================================================
>>> --- doc/sourcebuild.texi (revision 220617)
>>> +++ doc/sourcebuild.texi (working copy)
>>> @@ -1884,6 +1884,9 @@
>>>  @item nonpic
>>>  Target does not generate PIC by default.
>>>
>>> +@item pie_enabled
>>> +Target generates PIE by default.
>>> +
>>>  @item pcc_bitfield_type_matters
>>>  Target defines @code{PCC_BITFIELD_TYPE_MATTERS}.
>>>
>>> With -fpie, PIE is also enabled, just not the default without any
>>
>> I was testing
>>
>> # make RUNTESTFLAGS="--target_board='unix{-m32\ -fpie,-fpie}'
>>
>> I don't consider PIE is default.  It is just enabled.
>>
>>> options.  Please either go with the pie_default I sugested or wait for
>>> others to weigh in before rushing in another `obvious' fix.
>
> Then the description (both sourcebuild.texi and target-supports.texi) is
> confusing.

That is how other options are described.

> What are you trying to achieve here, actually?  Even on Solaris 11/x86
> (which doesn't support PIE), -fpie lets the
> check_effective_target_pie_enabled (or whatever it's called) proc pass.
> Shouldn't it also check if the target can support PIE at all?
>

Assembly outputs may be different, depending on if PIE is
enabled nor not. When we scan assembly outputs for test
results, we have different expected results when PIE is enabled.
That is how pie_enabled is used so far.
Rainer Orth Feb. 11, 2015, 4:15 p.m. UTC | #4
"H.J. Lu" <hjl.tools@gmail.com> writes:

> On Wed, Feb 11, 2015 at 7:19 AM, Rainer Orth
> <ro@cebitec.uni-bielefeld.de> wrote:
>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>>
>>> On Wed, Feb 11, 2015 at 6:10 AM, Rainer Orth
>>> <ro@cebitec.uni-bielefeld.de> wrote:
>>>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>>>>
>>>>>> The new proc is bogus, unfortunately: there's already an existing
>>>>>> check_effective_target_pie that checks if a target can support PIE.  The
>>>>>> new one just overrides the previous one.  On targets supporting PIE
>>>>>> (like Darwin), but not defaulting to it, the PIE tests suddenly turn out
>>>>>> UNSUPPORTED.
>>>>>>
>>>>>> You should rename the new one to
>>>>>> e.g. check_effective_target_pie_default, update the single user, and
>>>>>> document it in sourcebuild.texi.
>>>>>
>>>>> I checked in this as an obvious fix.
>>>>
>>>> I think pie_enabled is not a very descriptive name:
>>>>
>>>> Index: doc/sourcebuild.texi
>>>> ===================================================================
>>>> --- doc/sourcebuild.texi (revision 220617)
>>>> +++ doc/sourcebuild.texi (working copy)
>>>> @@ -1884,6 +1884,9 @@
>>>>  @item nonpic
>>>>  Target does not generate PIC by default.
>>>>
>>>> +@item pie_enabled
>>>> +Target generates PIE by default.
>>>> +
>>>>  @item pcc_bitfield_type_matters
>>>>  Target defines @code{PCC_BITFIELD_TYPE_MATTERS}.
>>>>
>>>> With -fpie, PIE is also enabled, just not the default without any
>>>
>>> I was testing
>>>
>>> # make RUNTESTFLAGS="--target_board='unix{-m32\ -fpie,-fpie}'
>>>
>>> I don't consider PIE is default.  It is just enabled.
>>>
>>>> options.  Please either go with the pie_default I sugested or wait for
>>>> others to weigh in before rushing in another `obvious' fix.
>>
>> Then the description (both sourcebuild.texi and target-supports.texi) is
>> confusing.
>
> That is how other options are described.

You're not getting my point:

* target-supports.exp now has

# Return 1 if the current multilib generates PIE by default.

* sourcebuild.texi states

Target generates PIE by default.

Which one is it?

>> What are you trying to achieve here, actually?  Even on Solaris 11/x86
>> (which doesn't support PIE), -fpie lets the
>> check_effective_target_pie_enabled (or whatever it's called) proc pass.
>> Shouldn't it also check if the target can support PIE at all?
>
> Assembly outputs may be different, depending on if PIE is
> enabled nor not. When we scan assembly outputs for test
> results, we have different expected results when PIE is enabled.
> That is how pie_enabled is used so far.

But why would you need a new effective-target keyword for that?  Simply
test the existing { target pie }, add -fpie and scan the assembler
output.  Why would you need pie_enabled or whatever at all?  Again: what
are you trying to achieve that cannot be done with the current keyword?

Right now, in gcc.target/i386/pie.c, you're only testing the PIE case
when PIE has been enabled by some means external to testsuite.  The test
always comes out UNSUPPORTED if not, so it isn't even exercised in a
regular bootstrap (except on Darwin which defaults to PIE, I believe).

	Rainer
H.J. Lu Feb. 11, 2015, 4:45 p.m. UTC | #5
On Wed, Feb 11, 2015 at 8:15 AM, Rainer Orth
<ro@cebitec.uni-bielefeld.de> wrote:
> "H.J. Lu" <hjl.tools@gmail.com> writes:
>
>> On Wed, Feb 11, 2015 at 7:19 AM, Rainer Orth
>> <ro@cebitec.uni-bielefeld.de> wrote:
>>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>>>
>>>> On Wed, Feb 11, 2015 at 6:10 AM, Rainer Orth
>>>> <ro@cebitec.uni-bielefeld.de> wrote:
>>>>> "H.J. Lu" <hjl.tools@gmail.com> writes:
>>>>>
>>>>>>> The new proc is bogus, unfortunately: there's already an existing
>>>>>>> check_effective_target_pie that checks if a target can support PIE.  The
>>>>>>> new one just overrides the previous one.  On targets supporting PIE
>>>>>>> (like Darwin), but not defaulting to it, the PIE tests suddenly turn out
>>>>>>> UNSUPPORTED.
>>>>>>>
>>>>>>> You should rename the new one to
>>>>>>> e.g. check_effective_target_pie_default, update the single user, and
>>>>>>> document it in sourcebuild.texi.
>>>>>>
>>>>>> I checked in this as an obvious fix.
>>>>>
>>>>> I think pie_enabled is not a very descriptive name:
>>>>>
>>>>> Index: doc/sourcebuild.texi
>>>>> ===================================================================
>>>>> --- doc/sourcebuild.texi (revision 220617)
>>>>> +++ doc/sourcebuild.texi (working copy)
>>>>> @@ -1884,6 +1884,9 @@
>>>>>  @item nonpic
>>>>>  Target does not generate PIC by default.
>>>>>
>>>>> +@item pie_enabled
>>>>> +Target generates PIE by default.
>>>>> +
>>>>>  @item pcc_bitfield_type_matters
>>>>>  Target defines @code{PCC_BITFIELD_TYPE_MATTERS}.
>>>>>
>>>>> With -fpie, PIE is also enabled, just not the default without any
>>>>
>>>> I was testing
>>>>
>>>> # make RUNTESTFLAGS="--target_board='unix{-m32\ -fpie,-fpie}'
>>>>
>>>> I don't consider PIE is default.  It is just enabled.
>>>>
>>>>> options.  Please either go with the pie_default I sugested or wait for
>>>>> others to weigh in before rushing in another `obvious' fix.
>>>
>>> Then the description (both sourcebuild.texi and target-supports.texi) is
>>> confusing.
>>
>> That is how other options are described.
>
> You're not getting my point:
>
> * target-supports.exp now has
>
> # Return 1 if the current multilib generates PIE by default.
>
> * sourcebuild.texi states
>
> Target generates PIE by default.
>
> Which one is it?

I copied them from nonpic

# Return 1 if the current multilib does not generate PIC by default.

@item nonpic
Target does not generate PIC by default.

>>> What are you trying to achieve here, actually?  Even on Solaris 11/x86
>>> (which doesn't support PIE), -fpie lets the
>>> check_effective_target_pie_enabled (or whatever it's called) proc pass.
>>> Shouldn't it also check if the target can support PIE at all?
>>
>> Assembly outputs may be different, depending on if PIE is
>> enabled nor not. When we scan assembly outputs for test
>> results, we have different expected results when PIE is enabled.
>> That is how pie_enabled is used so far.
>
> But why would you need a new effective-target keyword for that?  Simply
> test the existing { target pie }, add -fpie and scan the assembler
> output.  Why would you need pie_enabled or whatever at all?  Again: what
> are you trying to achieve that cannot be done with the current keyword?
>
> Right now, in gcc.target/i386/pie.c, you're only testing the PIE case
> when PIE has been enabled by some means external to testsuite.  The test
> always comes out UNSUPPORTED if not, so it isn't even exercised in a
> regular bootstrap (except on Darwin which defaults to PIE, I believe).

That is intentional.  gcc.target/i386/pie.c will be test by

1. RUNTESTFLAGS="--target_board='unix{-m32\ -fpie,-fpie}'" .  Or
2. PIE is enabled by default:

https://gcc.gnu.org/ml/gcc-patches/2015-01/msg00956.html
diff mbox

Patch

Index: doc/sourcebuild.texi
===================================================================
--- doc/sourcebuild.texi (revision 220617)
+++ doc/sourcebuild.texi (working copy)
@@ -1884,6 +1884,9 @@ 
 @item nonpic
 Target does not generate PIC by default.

+@item pie_enabled
+Target generates PIE by default.
+
 @item pcc_bitfield_type_matters
 Target defines @code{PCC_BITFIELD_TYPE_MATTERS}.