[testsuite] Adding target nonpic to g++.dg/tm/pr47746.C

Hi Patrick,

I'm trying it on linux/x86_64 on trunk. Testing just by adding -fpic
to the dg-options:

testsuite/g++.dg/tm/pr47746.C:20:14: error: unsafe function call 'void
Building::load(InputStream*)' within 'transaction_safe' function

And the backtrace:

#0  ipa_tm_diagnose_tm_safe (node=0x7ffff19a5940) at
src/gcc/gcc/trans-mem.c:4499
#1  0x0000000000c6364b in ipa_tm_execute () at src/gcc/gcc/trans-mem.c:5323
#2  0x0000000000b66bce in execute_one_pass (pass=0x1a46d00) at
src/gcc/gcc/passes.c:2331
#3  0x0000000000b6787c in execute_ipa_pass_list (pass=0x1a46d00) at
src/gcc/gcc/passes.c:2688

in the code:
4499|  for (e = node->callees; e ; e = e->next_callee)
4500|     if (!is_tm_callable (e->callee->symbol.decl)
4501|         && e->callee->local.tm_may_enter_irr)
4502+>       error_at (gimple_location (e->call_stmt),
4503|                 "unsafe function call %qD within "
4504|                 "%<transaction_safe%> function", e->callee->symbol.decl);

AFAIU, The eventual reason for that error_at, as I wrote before, is the check:

4461|   /* If we aren't seeing the final version of the function we don't
4462|      know what it will contain at runtime.  */
4463|   if (cgraph_function_body_availability (node) < AVAIL_AVAILABLE)
4464+>    return true;
4465|

(gdb) p cgraph_function_body_availability (node)
$54 = AVAIL_OVERWRITABLE

Sure I can file a PR If you think that the test was not supposed to
fail with -fpic

thanks,
Alexander




2013/4/18 Patrick Marlier <patrick.marlier@gmail.com>:
> Hi Alexander,
>
> On Thu, Apr 11, 2013 at 11:37 AM, Alexander Ivchenko <aivchenk@gmail.com> wrote:
>> The same motivation as for:
>> http://gcc.gnu.org/ml/gcc-cvs/2013-03/msg00786.html
>>
>> "Since -fpic option is turned on by default in Android we have certain test
>> fails. The reason for that is that those tests rely on the
>> availability of functions, defined in them
>> and with -fpic compiler conservatively assumes that they are
>> AVAIL_OVERWRITABLE."
>>
>> In case of tm we have that in here:
>>
>> 4461|   /* If we aren't seeing the final version of the function we don't
>> 4462|      know what it will contain at runtime.  */
>> 4463|   if (cgraph_function_body_availability (node) < AVAIL_AVAILABLE)
>> 4464+>    return true;
>> 4465|
>>
>> (gdb) p cgraph_function_body_availability (node)
>> $54 = AVAIL_OVERWRITABLE
>>
>> and so we have a testfail for Android.
>
>
> Where/how does it fails? (backtrace?) I cannot reproduce with -fpic on
> linux/x86.
>
> Actually the test is not supposed to fail even with pic. So maybe you
> should open a PR.
>
> Thanks,
> --
> Patrick Marlier

Hi Alexander,

On Thu, Apr 18, 2013 at 12:49 PM, Alexander Ivchenko <aivchenk@gmail.com> wrote:
> I'm trying it on linux/x86_64 on trunk. Testing just by adding -fpic
> to the dg-options:
>
> --- a/gcc/testsuite/g++.dg/tm/pr47746.C
> +++ b/gcc/testsuite/g++.dg/tm/pr47746.C
> @@ -1,5 +1,5 @@
>  // { dg-do compile }
> -// { dg-options "-fgnu-tm" }
> +// { dg-options "-fgnu-tm -fpic" }
>
> Here is the error msg:
>
> testsuite/g++.dg/tm/pr47746.C:20:14: error: unsafe function call 'void
> Building::load(InputStream*)' within 'transaction_safe' function

Thanks! Now I understand the error (and I am able to reproduce it). :)

> 4462|      know what it will contain at runtime.  */
> 4463|   if (cgraph_function_body_availability (node) < AVAIL_AVAILABLE)
> 4464+>    return true;
> 4465|
>
> (gdb) p cgraph_function_body_availability (node)
> $54 = AVAIL_OVERWRITABLE
>
> Sure I can file a PR If you think that the test was not supposed to
> fail with -fpic

I think your patch is OK but I cannot approve it since I am not a maintainer.
I CCed Richard since he is the one who can approve and knows the TM
implementation.

Thanks,
--
Patrick

*ping*

Alexander

2013/4/18 Patrick Marlier <patrick.marlier@gmail.com>:
> Hi Alexander,
>
> On Thu, Apr 18, 2013 at 12:49 PM, Alexander Ivchenko <aivchenk@gmail.com> wrote:
>> I'm trying it on linux/x86_64 on trunk. Testing just by adding -fpic
>> to the dg-options:
>>
>> --- a/gcc/testsuite/g++.dg/tm/pr47746.C
>> +++ b/gcc/testsuite/g++.dg/tm/pr47746.C
>> @@ -1,5 +1,5 @@
>>  // { dg-do compile }
>> -// { dg-options "-fgnu-tm" }
>> +// { dg-options "-fgnu-tm -fpic" }
>>
>> Here is the error msg:
>>
>> testsuite/g++.dg/tm/pr47746.C:20:14: error: unsafe function call 'void
>> Building::load(InputStream*)' within 'transaction_safe' function
>
> Thanks! Now I understand the error (and I am able to reproduce it). :)
>
>> 4462|      know what it will contain at runtime.  */
>> 4463|   if (cgraph_function_body_availability (node) < AVAIL_AVAILABLE)
>> 4464+>    return true;
>> 4465|
>>
>> (gdb) p cgraph_function_body_availability (node)
>> $54 = AVAIL_OVERWRITABLE
>>
>> Sure I can file a PR If you think that the test was not supposed to
>> fail with -fpic
>
> I think your patch is OK but I cannot approve it since I am not a maintainer.
> I CCed Richard since he is the one who can approve and knows the TM
> implementation.
>
> Thanks,
> --
> Patrick

Oh, sorry. Missed the last msg from Mike in that thread.

2013/5/27 Alexander Ivchenko <aivchenk@gmail.com>:
> *ping*
>
> Alexander
>
> 2013/4/18 Patrick Marlier <patrick.marlier@gmail.com>:
>> Hi Alexander,
>>
>> On Thu, Apr 18, 2013 at 12:49 PM, Alexander Ivchenko <aivchenk@gmail.com> wrote:
>>> I'm trying it on linux/x86_64 on trunk. Testing just by adding -fpic
>>> to the dg-options:
>>>
>>> --- a/gcc/testsuite/g++.dg/tm/pr47746.C
>>> +++ b/gcc/testsuite/g++.dg/tm/pr47746.C
>>> @@ -1,5 +1,5 @@
>>>  // { dg-do compile }
>>> -// { dg-options "-fgnu-tm" }
>>> +// { dg-options "-fgnu-tm -fpic" }
>>>
>>> Here is the error msg:
>>>
>>> testsuite/g++.dg/tm/pr47746.C:20:14: error: unsafe function call 'void
>>> Building::load(InputStream*)' within 'transaction_safe' function
>>
>> Thanks! Now I understand the error (and I am able to reproduce it). :)
>>
>>> 4462|      know what it will contain at runtime.  */
>>> 4463|   if (cgraph_function_body_availability (node) < AVAIL_AVAILABLE)
>>> 4464+>    return true;
>>> 4465|
>>>
>>> (gdb) p cgraph_function_body_availability (node)
>>> $54 = AVAIL_OVERWRITABLE
>>>
>>> Sure I can file a PR If you think that the test was not supposed to
>>> fail with -fpic
>>
>> I think your patch is OK but I cannot approve it since I am not a maintainer.
>> I CCed Richard since he is the one who can approve and knows the TM
>> implementation.
>>
>> Thanks,
>> --
>> Patrick

Hi,

This is just a memo about why the testcase failed:

When it tries to compile, we get this:
gcc/testsuite/g++.dg/tm/pr47746.C:20:14: error: unsafe function call
‘void Building::load(InputStream*)’ within ‘transaction_safe’ function
  load(stream);

Indeed, with PIC, the 'load' method can be overloaded later so we
cannot infer that the body will be transaction_safe.
in trans-mem.c:
4461|   /* If we aren't seeing the final version of the function we don't
4462|      know what it will contain at runtime.  */
4463|   if (cgraph_function_body_availability (node) < AVAIL_AVAILABLE)
4464+>    return true;

The two ways to fix this, is either to force for non-pic target or to
add transaction_safe attribute to 'load' and 'readUint32' methods.
--
Patrick