Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: #1690 unable to identify shebang when it has initial spaces #1696

Closed
wants to merge 5 commits into from
Closed

fix: #1690 unable to identify shebang when it has initial spaces #1696

wants to merge 5 commits into from

Conversation

yihong0618
Copy link
Contributor

@yihong0618 yihong0618 commented May 10, 2023

fix: #1690
Signed-off-by: yihong0618 [email protected]

Copy link
Owner

@namhyung namhyung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please squash the commits into one and rewrite the commit message like this?

record: skip whitespaces after shebang for scripts

Python tracing won't work when the shebang line has a space like below:

    #! /usr/bin/python3

Fixed: #1690 

Signed-off-by: ...

cmds/record.c Outdated
@@ -19,6 +19,7 @@
#include <sys/wait.h>
#include <unistd.h>

#include "libtraceevent/event-utils.h"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to include libtraceevent headers just because of this. Maybe we can implement strim() instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do this tonight

@yihong0618
Copy link
Contributor Author

Can you please squash the commits into one and rewrite the commit message like this?

record: skip whitespaces after shebang for scripts

Python tracing won't work when the shebang line has a space like below:

    #! /usr/bin/python3

Fixed: #1690 

Signed-off-by: ...

of course

namhyung and others added 5 commits May 10, 2023 14:01
  gcc -std=gnu11 -D_GNU_SOURCE   -iquote /localdisk/uftrace -iquote /localdisk/uftrace \
    -iquote /localdisk/uftrace/arch/x86_64 -Wdeclaration-after-statement -W -Wall \
    -Wno-unused-parameter -Wno-missing-field-initializers -O0 -g3 -DDEBUG_MODE=1 \
    -DHAVE_CXA_DEMANGLE -DHAVE_LIBPYTHON3 -I/usr/include/python3.6m   \
    -DLIBPYTHON_VERSION=3.6 -DHAVE_PERF_CLOCKID -DHAVE_PERF_CTXSW -DHAVE_LIBNCURSES \
    -DHAVE_LIBELF -DHAVE_LIBDW   -DHAVE_LIBUNWIND   -DHAVE_LIBCAPSTONE -I/usr/include/capstone \
    -fPIC -c -o /localdisk/uftrace/python/trace-python.op /localdisk/uftrace/python/trace-python.c
  /localdisk/uftrace/python/trace-python.c: In function ‘get_python_funcname’:
  /localdisk/uftrace/python/trace-python.c:760:7: warning: ‘main’ is usually a function [-Wmain]
    bool main = false;
         ^

Reported-by: Clément Guidi <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
It should return NULL if it fails to get the symbol address.
And it needs to check the return value before using symbol.

Signed-off-by: Namhyung Kim <[email protected]>
C functions get the code object from the argument not from the frame.
Also it overwrote the code after getting the reference.

Signed-off-by: Namhyung Kim <[email protected]>
It keeps the code objects to find a matching symbol for the given
function.  Let's handle them properly for GC to check references.

Likewise, it should clear the reference in the tp_clear callback.

Signed-off-by: Namhyung Kim <[email protected]>
Python tracing won't work when the shebang line has a space like below:

    #! /usr/bin/python3

Fixed: #1690

Signed-off-by: yihong0618 <[email protected]>
@yihong0618 yihong0618 closed this May 10, 2023
@namhyung
Copy link
Owner

Next time, you can reuse this PR instead of creating a new one. You don't need to create a new PR whenever you update the code. Just force-push is fine.

@yihong0618
Copy link
Contributor Author

yihong0618 commented May 11, 2023

copy that, thanks
caused the Signed-off and rebase make this PR a little messy
I will do it better next time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unable to identify shebang when it has initial spaces
2 participants