Hey there,

ok, tracing this a bit further, I have some more questions :-)

If I am not mistaken, the code around Parser.C, line 1013 is supposed to
deal with calls to non-returning targets?

is_nonret = obj().cs()->nonReturning(target);
if (is_nonret) {
 ....

Now, in my example, for some reason the .plt.got stub function that jmps
straight to cs:__stack_chk_fail is not a
function:

.plt.got:0000000000001030 __stack_chk_fail proc near              ; CODE
XREF: postproc_version:loc_11D0↓p
.plt.got:0000000000001030                                         ;
postproc_configuration:loc_1202↓p ...
.plt.got:0000000000001030                 jmp     cs:__stack_chk_fail_ptr
.plt.got:0000000000001030 __stack_chk_fail endp

So when nonReturning(0x1030) is called, no function gets returned in
SymtabCodeSource::nonReturning while
calling findFuncByEntryOffset, and then the code mistakenly adds the
fallthrough edge.

I am still finding my way through the codebase - but I presume there is
some initial pass that is supposed to create
functions from the stubs in the PLT and check them for non-returning status?

Cheers,
Thomas



On Thu, Mar 15, 2018 at 6:33 PM, Thomas Dullien <thomasdull...@google.com>
wrote:

> Hey there,
>
> one quick question: After backporting the 3 commits, I tested with both
> recursive-mode disassembly and non-recursive mode
> disassembly, and neither of the two worked. My question is now: Is this
> even supposed to work in non-recursive mode? E.g.
> if we are not recursing into a called function, we can't tell whether it
> returns or not, right?
>
> Cheers,
> Thomas
>
> On Thu, Mar 15, 2018 at 2:23 PM, Thomas Dullien <thomasdull...@google.com>
> wrote:
>
>> Hey there,
>>
>> ah, cool :-). I backported the 3 commits into the 9.3.2 codebase, but it
>> does not seem to solve the issue just yet.
>>
>> I will debug a bit more and report back once I understand what is going
>> on.
>>
>> Cheers,
>> Thomas
>>
>> On Thu, Mar 15, 2018 at 2:09 PM, Xiaozhu Meng <xm...@cs.wisc.edu> wrote:
>>
>>> Ah, I have a few pending commits related to this issue. Please look at
>>> my pull request at https://github.com/dyninst/dyninst/pull/437.
>>>
>>> Not all commits are relevant to you, but bbac557, 78b1bd1, da50a37 should
>>> be relevant.
>>>
>>> For parsing_printf, it is "DYNINST_DEBUG_PARSING".
>>>
>>>
>>> On Thu, Mar 15, 2018 at 8:02 AM, Thomas Dullien <
>>> thomasdull...@google.com> wrote:
>>>
>>>> Hey there,
>>>>
>>>> ok, rebuilding DynInst 9.3.2 now to investigate why adding the string
>>>> does not seem to have any effect.
>>>> I looked at the source code, and I *think* the function is already in
>>>> the list of non-returning functions.
>>>>
>>>> Checking what is going on. Example setup:
>>>>
>>>> .plt.got:0000000000053CF8 __stack_chk_fail proc near              ;
>>>> CODE XREF: init:loc_54673↓p
>>>> .plt.got:0000000000053CF8                                         ;
>>>> uninit:loc_54705↓p ...
>>>> .plt.got:0000000000053CF8                 jmp
>>>>  cs:__stack_chk_fail_ptr
>>>> .plt.got:0000000000053CF8 __stack_chk_fail endp
>>>>
>>>> .text:00000000000A4290 var_8           = qword ptr -8
>>>> .text:00000000000A4290
>>>> .text:00000000000A4290 graph = rdi                             ;
>>>> AVFilterGraph_0 *
>>>> .text:00000000000A4290 flags = rsi                             ;
>>>> unsigned int
>>>> .text:00000000000A4290 ; __unwind {
>>>> .text:00000000000A4290                 push    rax
>>>> .text:00000000000A4291                 mov     rax, fs:28h
>>>> .text:00000000000A429A                 mov     [rsp+8+var_8], rax
>>>> .text:00000000000A429E                 mov     [graph+5Ch], esi
>>>> .text:00000000000A42A1                 mov     rax, fs:28h
>>>> .text:00000000000A42AA                 cmp     rax, [rsp+8+var_8]
>>>> .text:00000000000A42AE                 jnz     short loc_A42B2
>>>> .text:00000000000A42B0                 pop     rax
>>>> .text:00000000000A42B1                 retn
>>>> .text:00000000000A42B2 ; ------------------------------
>>>> ---------------------------------------------
>>>> .text:00000000000A42B2
>>>> .text:00000000000A42B2 loc_A42B2:                              ; CODE
>>>> XREF: avfilter_graph_set_auto_convert+1E↑j
>>>> .text:00000000000A42B2                 call    __stack_chk_fail
>>>> .text:00000000000A42B2 ; } // starts at A4290
>>>> .text:00000000000A42B2 avfilter_graph_set_auto_convert endp
>>>>
>>>> DynInst for some reason does not interpret the tailing call as
>>>> non-return. Digging to see what is going on,
>>>> will update here as I learn more :)
>>>>
>>>> Cheers,
>>>> Thomas
>>>>
>>>>
>>>> On Thu, Mar 15, 2018 at 1:58 PM, Xiaozhu Meng <mxz...@gmail.com> wrote:
>>>>
>>>>> Hi Thomas,
>>>>>
>>>>> On Thu, Mar 15, 2018 at 4:06 AM, Thomas Dullien <
>>>>> thomasdull...@google.com> wrote:
>>>>>
>>>>>> Hey there,
>>>>>>
>>>>>> ok, I have looked at a few options on how to best tackle this, and
>>>>>> would love to solicit advice :-)
>>>>>>
>>>>>> - I tried SymtabCodeSource::addNonReturning("__stack_chk_fail");
>>>>>> this did not seem to have an effect.
>>>>>>
>>>>>
>>>>> If you call SymtabCodeSource::addNonReturning("__stack_chk_fail")
>>>>> before calling CodeObject::parse(), this should work.
>>>>>
>>>>>
>>>>>> - Looked at set_retstatus -- but that implies that the code is
>>>>>> already parsed?
>>>>>>
>>>>>
>>>>> You are right that after CodeObject::parse() has finished, calling
>>>>> set_retstatus will only change the flag of return status for this 
>>>>> function,
>>>>> will not re-parse the binary. So, we can focus on why
>>>>> SymtabCodeSource::addNonReturning("__stack_chk_fail") does not work.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> Cheers,
>>>>>> Thomas
>>>>>>
>>>>>>
>>>>>> On Thu, Mar 15, 2018 at 9:28 AM, Thomas Dullien <
>>>>>> thomasdull...@google.com> wrote:
>>>>>>
>>>>>>> Ah. No. I just fund set_retstatus :-) -- please ignore my question
>>>>>>> :-)
>>>>>>>
>>>>>>> On Thu, Mar 15, 2018 at 9:26 AM, Thomas Dullien <
>>>>>>> thomasdull...@google.com> wrote:
>>>>>>>
>>>>>>>> Hey there,
>>>>>>>>
>>>>>>>> after having my coffee, I realized: The proper way to do this is to
>>>>>>>> derive from CodeSource
>>>>>>>> and overload the nonReturning functions, I guess? :)
>>>>>>>>
>>>>>>>> Cheers,
>>>>>>>> Thomas
>>>>>>>>
>>>>>>>> On Thu, Mar 15, 2018 at 9:14 AM, Thomas Dullien <
>>>>>>>> thomasdull...@google.com> wrote:
>>>>>>>>
>>>>>>>>> Hey there,
>>>>>>>>>
>>>>>>>>> I am running into troubles with disassembling executables
>>>>>>>>> generated by
>>>>>>>>> clang.3.8.1-24, for x64, with optimization set to size-optimize
>>>>>>>>> and stack cookies
>>>>>>>>> enabled.
>>>>>>>>>
>>>>>>>>> The trouble is that any function with an enabled stack cookie will
>>>>>>>>> end with a sequence
>>>>>>>>> of:
>>>>>>>>>
>>>>>>>>>   Epilogue to check stack cookie
>>>>>>>>>   jnz .fail
>>>>>>>>>   Rest of epilogue.
>>>>>>>>>   retn
>>>>>>>>> .fail:
>>>>>>>>>   call __stack_checkfail     // Does not return
>>>>>>>>>
>>>>>>>>> This leads to DynInst lumping all consecutive functions that use
>>>>>>>>> stack cookies
>>>>>>>>> into one huge CFG.
>>>>>>>>>
>>>>>>>>> Is there a way to tell DynInst that a particular function is not
>>>>>>>>> returning? I found
>>>>>>>>> that the parseAPI allows me to query if a function returns, but I
>>>>>>>>> did not find anything
>>>>>>>>> to "override" this behavior?
>>>>>>>>>
>>>>>>>>> Cheers,
>>>>>>>>> Thomas
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Dyninst-api mailing list
>>>>>> Dyninst-api@cs.wisc.edu
>>>>>> https://lists.cs.wisc.edu/mailman/listinfo/dyninst-api
>>>>>>
>>>>>
>>>>>
>>>>
>>>> _______________________________________________
>>>> Dyninst-api mailing list
>>>> Dyninst-api@cs.wisc.edu
>>>> https://lists.cs.wisc.edu/mailman/listinfo/dyninst-api
>>>>
>>>
>>>
>>
>
_______________________________________________
Dyninst-api mailing list
Dyninst-api@cs.wisc.edu
https://lists.cs.wisc.edu/mailman/listinfo/dyninst-api

Reply via email to