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

Full tracepoint support in Clang front-end #602

Merged
merged 4 commits into from
Jul 11, 2016

Conversation

goldshtn
Copy link
Collaborator

@goldshtn goldshtn commented Jul 8, 2016

This should close #587 by providing full support for tracepoints in the Clang front-end. Namely, probe functions for tracepoints can now be defined using the TRACEPOINT_PROBE macro. For example:

TRACEPOINT_PROBE(sched, sched_switch) {
  bpf_trace_printk("%d -> %d\n", args->prev_pid, args->next_pid);
  return 0;
}

@goldshtn goldshtn mentioned this pull request Jul 8, 2016
3 tasks
@goldshtn
Copy link
Collaborator Author

goldshtn commented Jul 8, 2016

@4ast @drzaeus77: A bunch of tests are failing on the build bots with the following error:

11: terminate called after throwing an instance of 'std::regex_error'
11:   what():  regex_error

I know I introduced regexes in this PR, but ... it works on my machine. All tests are passing. This could be due to the libstdc++ version, or I'm not sure what else. Is there any way to debug what's happening on the build bots? Get a complete stack trace of the exception?

Edited to add: It looks like some older versions of gcc might have had incomplete support for <regex>. Specifically, it might be the \s or \S causing them trouble. I don't see the compiler version in the build log, so not sure how to confirm.

@drzaeus77
Copy link
Collaborator

Actually, fedora buildbot got a different error. Here is the backtrace of the ubuntu one:

Program received signal SIGABRT, Aborted.
0x00007ffff69c7c37 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
56      ../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  0x00007ffff69c7c37 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x00007ffff69cb028 in __GI_abort () at abort.c:89
#2  0x00007ffff72d2535 in __gnu_cxx::__verbose_terminate_handler() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#3  0x00007ffff72d06d6 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#4  0x00007ffff72d0703 in std::terminate() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#5  0x00007ffff72d0922 in __cxa_throw () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#6  0x00007ffff7322955 in std::__throw_regex_error(std::regex_constants::error_type) () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#7  0x00000000007e8b58 in std::__detail::_Compiler<char const*, std::regex_traits<char> >::_M_match_token(std::__detail::_Scanner<char const*>::_TokenT) [clone .part.1507] ()
#8  0x00000000007ee3ed in std::__detail::_Compiler<char const*, std::regex_traits<char> >::_M_atom() ()
#9  0x00000000007ee67a in std::__detail::_Compiler<char const*, std::regex_traits<char> >::_M_alternative() ()
#10 0x00000000007ee844 in std::__detail::_Compiler<char const*, std::regex_traits<char> >::_M_disjunction() ()
#11 0x00000000007eec33 in std::__detail::_Compiler<char const*, std::regex_traits<char> >::_Compiler(char const* const&, char const* const&, std::regex_traits<char>&, unsigned int) ()
#12 0x00000000007ef1d8 in std::shared_ptr<std::__detail::_Automaton> std::__detail::__compile<char const*, std::regex_traits<char> >(char const* const&, char const* const&, std::regex_traits<char>&, unsigned int) ()
#13 0x00000000007e8c36 in ebpf::TracepointTypeVisitor::VisitFunctionDecl(clang::FunctionDecl*) ()
#14 0x00000000007e8dc5 in clang::RecursiveASTVisitor<ebpf::TracepointTypeVisitor>::TraverseDecl(clang::Decl*) [clone .part.2166] ()
#15 0x00000000007e94ee in ebpf::TracepointTypeConsumer::HandleTopLevelDecl(clang::DeclGroupRef) ()
#16 0x00000000009d1956 in clang::ParseAST(clang::Sema&, bool, bool) ()
#17 0x0000000000822746 in clang::FrontendAction::Execute() ()
#18 0x00000000008012d8 in clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) ()
#19 0x00000000007919ac in ebpf::ClangLoader::parse(std::unique_ptr<llvm::Module, std::default_delete<llvm::Module> >*, std::unique_ptr<std::vector<ebpf::TableDesc, std::allocator<ebpf::TableDesc> >, std::default_delete<std::vector<ebpf::TableDesc, std::allocator<ebpf::TableDesc> > > >*, std::string const&, bool, char const**, int) ()
#20 0x00000000007511f3 in ebpf::BPFModule::load_cfile(std::string const&, bool, char const**, int) ()
#21 0x0000000000756bfb in ebpf::BPFModule::load_string(std::string const&, char const**, int) ()
#22 0x0000000000750744 in bpf_module_create_c_from_string ()
#23 0x0000000000735547 in main ()

@goldshtn
Copy link
Collaborator Author

goldshtn commented Jul 8, 2016

@drzaeus77 Thanks for the stack trace, it looks like this libstdc++ version thinks the regex is invalid. I haven't seen something like this before, would appreciate any suggestions.

@goldshtn
Copy link
Collaborator Author

goldshtn commented Jul 8, 2016

The update I just pushed should fix the issue on the Fedora bot, I accidentally removed the len(open_kprobes) == 0 test in the probe auto-loader. By the way, it looks like depending on it is kind of buggy. The second and third tests in test_histogram.py don't call attach_kprobe, so the probe remains attached to the function from the first test, and just happens to pass the test because it's not asserting anything. The cleanup, which is supposed to detach the probe, runs in an @atexit handler, which isn't called between the tests because they are all in the same run. Is that something worth fixing?

@goldshtn
Copy link
Collaborator Author

goldshtn commented Jul 8, 2016

@drzaeus77: it would be great to see what the error type was in frame 6.

@brendangregg
Copy link
Member

Tested, works, thanks!

I'll update the examples/tracing/urandomread.py to be this:

#!/usr/bin/python
#[...]

from __future__ import print_function
from bcc import BPF

# load BPF program
b = BPF(text="""
TRACEPOINT_PROBE(random, urandom_read) {
    bpf_trace_printk("%d\\n", args->got_bits);
    return 0;
};
""")

# header
print("%-18s %-16s %-6s %s" % ("TIME(s)", "COMM", "PID", "GOTBITS"))

# format output
while 1:
    try:
        (task, pid, cpu, flags, ts, msg) = b.trace_fields()
    except ValueError:
        continue
    print("%-18.9f %-16s %-6d %s" % (ts, task, pid, msg))

Pretty nice!

I'm wondering if I should keep the old version of urandomread.py somewhere, since it still works, and demonstrates a different method that might be useful in some unforeseen case.

@drzaeus77
Copy link
Collaborator

it would be great to see what the error type was in frame 6.

Tried, wasn't able to get it. However, I did reproduce with the following minimal program:

// g++ x.cc -std=c++11; ./a.out
#include <regex>
using namespace std;
int main(int argc, char **argv) {
  regex type_regex("(?:struct|class)\\s+tracepoint__(\\S+)__(\\S+)");
  return 0;
}

Test machine was laptop with Ubuntu 14.04. Nothing else special.

Taking a different tact, though, might be to avoid regex altogether. If I read the code right, you are using regex to determine when a type is a tracepoint arg.

I have a similar problem with the packet parsing rewrites that I get when using structs from proto.h. In there, I define a struct with the trailer BPF_PACKET_HEADER. This adds a meaningless (to the syntax) attribute to the type, which I then search for in the frontend action. I can do this in an exact way, since the type is reachable from the variable declaration. That then triggers the rewrite logic.

Have you noticed this infra, and considered it as an alternative?

@goldshtn
Copy link
Collaborator Author

goldshtn commented Jul 9, 2016

@drzaeus77: The thing is that I am using regexes to actually extract data from these strings. If I simply wanted a marker, I guess the __attribute__(deprecated("...")) solution could suffice. But I'm using regexes in two places:

  1. To determine that a function has a parameter of type struct tracepoint__category__event *
  2. To parse the tracepoint format file and obtain the data fields

I guess there isn't much choice though, and the older Ubuntu libstdc++ doesn't support std::regex. I'll rewrite the relevant pieces to use hand-written parsing instead.

When a function in the BPF program starts with "tracepoint__", parse
the rest of the name as a tracepoint category and name and attach the
tracepoint automatically. For example:

```
int tracepoint__sched__sched_switch(...)
```

As a result, the sched:sched_switch tracepoint is enabled and the function
is attached to that tracepoint.
When a probe function refers to a tracepoint arguments structure,
such as `struct tracepoint__irq__irq_handler_entry`, add that structure
on-the-fly using a Clang frontend action that runs before any other
steps take place.

Typically, the user will create tracepoint probe functions using
the TRACEPOINT_PROBE macro, which avoids the need for specifying
the tracepoint category and event twice in the signature of the
probe function.
@goldshtn
Copy link
Collaborator Author

goldshtn commented Jul 9, 2016

@drzaeus77: The latest commit moves from the std::regex stuff to manual parsing. The Ubuntu bot is still unhappy, but this is something else entirely --

usage: setup.py [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
   or: setup.py --help [cmd1 cmd2 ...]
   or: setup.py --help-commands
   or: setup.py cmd --help

error: option --root not recognized

[...]

dpkg-buildpackage: error: fakeroot debian/rules binary gave error exit status 2
debuild: fatal error at line 1364:
dpkg-buildpackage -rfakeroot -D -us -uc failed

Is it related to my PR at all?

@goldshtn
Copy link
Collaborator Author

@drzaeus77: I'd really appreciate if you could take a look, I'm really not sure what's failing the build here.

@drzaeus77
Copy link
Collaborator

ACK, will look at in a bit, still in weekend mode :)
On Jul 10, 2016 10:06 AM, "Sasha Goldshtein" [email protected]
wrote:

@drzaeus77 https://github.com/drzaeus77: I'd really appreciate if you
could take a look, I'm really not sure what's failing the build here.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#602 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ADmIvBxq_GdsHNoDecV7zVoP8zlu8friks5qUSahgaJpZM4JIQBe
.

@drzaeus77
Copy link
Collaborator

I guess it was just an intermittent issue.

if (D->isExternallyVisible() && D->hasBody()) {
// If this function has a tracepoint structure as an argument,
// add that structure declaration based on the structure name.
for (auto arg : D->params()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is going to have the same problem as was fixed in #600. Can you modify accordingly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@drzaeus77: Done. Thanks.

Older versions of GCC don't support std::regex even though they support
most of C++11. To avoid breaking the build on older systems, such as
Ubuntu 14.04, use manual parsing instead of std::regex.
@4ast
Copy link
Member

4ast commented Jul 11, 2016

lgtm

@drzaeus77 drzaeus77 merged commit 7452080 into iovisor:master Jul 11, 2016
@brendangregg brendangregg mentioned this pull request Jul 22, 2016
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.

Proper tracepoint support in BCC
4 participants