Hi Richard,

I also ran the gcc testsuite with
RUNTESTFLAGS="--tool_opts=-mcopyrelocs" to check for issues.  The only
test that failed was g++.dg/tsan/default_options.C.  It uses -fpie
-pie and BFD ld to link. Since BFD ld does not support copy
relocations with -pie, it does not link. I linked with gold to make
the test pass.

Could you please take another look at this patch?

Thanks
Sri

On Mon, Sep 8, 2014 at 3:19 PM, Sriraman Tallam <tmsri...@google.com> wrote:
> On Tue, Sep 2, 2014 at 1:40 PM, Richard Henderson <r...@redhat.com> wrote:
>> On 06/20/2014 05:17 PM, Sriraman Tallam wrote:
>>> Index: config/i386/i386.c
>>> ===================================================================
>>> --- config/i386/i386.c        (revision 211826)
>>> +++ config/i386/i386.c        (working copy)
>>> @@ -12691,7 +12691,9 @@ legitimate_pic_address_disp_p (rtx disp)
>>>               return true;
>>>           }
>>>         else if (!SYMBOL_REF_FAR_ADDR_P (op0)
>>> -                && SYMBOL_REF_LOCAL_P (op0)
>>> +                && (SYMBOL_REF_LOCAL_P (op0)
>>> +                    || (TARGET_64BIT && ix86_copyrelocs && flag_pie
>>> +                        && !SYMBOL_REF_FUNCTION_P (op0)))
>>>                  && ix86_cmodel != CM_LARGE_PIC)
>>>           return true;
>>>         break;
>>
>> This is the wrong place to patch.
>>
>> You ought to be adjusting SYMBOL_REF_LOCAL_P, by providing a modified
>> TARGET_BINDS_LOCAL_P.
>
> I have done this in the new attached patch, I added a new function
> i386_binds_local_p which will check for this and call
> default_binds_local_p otherwise.
>
>>
>> Note in particular that I believe that you are doing the wrong thing with 
>> weak
>> and COMMON symbols, in that you probably ought not force a copy reloc there.
>
> I added an extra check to not do this for WEAK symbols. I also added a
> check for DECL_EXTERNAL so I believe this will also not be called for
> COMMON symbols.
>
>>
>> Note the complexity of default_binds_local_p_1, and the fact that all you
>> really want to modify is
>>
>>   /* If PIC, then assume that any global name can be overridden by
>>      symbols resolved from other modules.  */
>>   else if (shlib)
>>     local_p = false;
>>
>> near the bottom of that function.
>
> I did not understand what you mean here? Were you suggesting an
> alternative way of doing this?
>
> Thanks for reviewing
> Sri
>
>>
>>
>> r~

Reply via email to