https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65400
--- Comment #7 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
(In reply to Jakub Jelinek from comment #6)
> Both patches look wrong to me.
> For the first change, it is wrong to add TSAN_FUNC_EXIT (), you should never
> add it out of nothing. First of all, you might consider allowing
> TSAN_FUNC_EXIT () in find_return_bb - there is no reason why any harm would
> be done if it is considered a part of a return bb. On your first testcase
> that is not the case though, so instead you need to either duplicate or move
> it. I'd say best would be to bail out early with fnsplitting if
> TSAN_FUNC_EXIT is present in a bb that is not return_bb itself or one of its
> predecessors; or when it is present in one of the predecessors of return_bb
> and not in all the other predecessors. The case when TSAN_FUNC_EXIT is in
> the return_bb (after you change find_return_bb) should work fine without any
> extra work, and for the case when it is in the predecessors of return_bb,
> add it.
>
Would you have time to continue on this?
> The second change doesn't make any sense at all, but from the testcase it
> isn't obvious what you are trying to do at all. If the problem is that
> fnsplit has set tail call flag and you've added the TSAN_FUNC_EXIT after it,
> then that should be where you clear the flag; if it is something different,
> please explain what you are trying to do and why.
fnsplit does _not_ set the tail call flag, but tail call optimization is
nevertheless happening. One other thing, that is probably too risky:
If for whatever reason the tail call optimization is not happening here,
the call stack would be incomplete, unfortunately the interesting part
would be missing. So I agree, the first patch may work for my application,
but, ....
The second test case has nothing to do with fnsplit, all I can tell is,
that these functions are binary identical, but operate on different
types, and these look in gimple like
OpcUa_Int32_P_NativeToWire (OpcUa_Int32_Wire * wire, OpcUa_Int32 * native)
{
OpcUa_StatusCode retval.4;
<bb 2>:
retval.4_5 = OpcUa_Float_P_NativeToWire (wire_2(D), native_3(D)); [tail call]
return retval.4_5;
}
so no TSAN_FUNC_EXIT at all.
with the patch that is transformed to
OpcUa_Int32_P_NativeToWire (OpcUa_Int32_Wire * wire, OpcUa_Int32 * native)
{
OpcUa_StatusCode retval.4;
void * _6;
<bb 2>:
_6 = __builtin_return_address (0);
__builtin___tsan_func_entry (_6);
retval.4_5 = OpcUa_Float_P_NativeToWire (wire_2(D), native_3(D));
__builtin___tsan_func_exit ();
return retval.4_5;
}
but without the patch this is transformed into:
OpcUa_Int32_P_NativeToWire (OpcUa_Int32_Wire * wire, OpcUa_Int32 * native)
{
OpcUa_StatusCode retval.4;
void * _6;
<bb 2>:
_6 = __builtin_return_address (0);
__builtin___tsan_func_entry (_6);
retval.4_5 = OpcUa_Float_P_NativeToWire (wire_2(D), native_3(D)); [tail call]
__builtin___tsan_func_exit ();
return retval.4_5;
}
This is probably incorrect gimple, because the former "tail call" is
no longer a tail call, because we add the __builtin__tsan_func_exit?
And the generated code jumps to OpcUa_Float_P_NativeToWire
and skips the call to __builtin___tsan_func_exit.
So the rationale of the patch in tsan.c is, if we find any call, here,
it will certainly not be a tail call any more, thus I think
resetting that flag unconditionally here seems to be OK.
