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.

Sign up for GitHub

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

Jump to bottom

Add 'get_helper_error_msg' utility #2905

Merged
merged 1 commit into from
Jan 15, 2024
Merged

Add 'get_helper_error_msg' utility #2905

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ and this project adheres to
- [#2830](https://github.com/iovisor/bpftrace/pull/2830)
- New builtin for getting the number of map elements
- [#2840](https://github.com/iovisor/bpftrace/pull/2840)
- Add more helpful error messages for map operations
- [#2905](https://github.com/iovisor/bpftrace/pull/2905)
#### Changed
#### Deprecated
#### Removed
Expand Down
10 changes: 1 addition & 9 deletions src/bpftrace.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,6 @@
#include "triggers.h"
#include "utils.h"

namespace libbpf {
#define __BPF_NAME_FN(x) #x
const char *bpf_func_name[] = { __BPF_FUNC_MAPPER(__BPF_NAME_FN) };
#undef __BPF_NAME_FN
} // namespace libbpf

namespace bpftrace {

DebugLevel bt_debug = DebugLevel::kNone;
Expand Down Expand Up @@ -542,9 +536,7 @@ void perf_event_printer(void *cb_cookie, void *data, int size)
auto error_id = helpererror->error_id;
auto return_value = helpererror->return_value;
auto &info = bpftrace->resources.helper_error_info[error_id];
bpftrace->out_->helper_error(libbpf::bpf_func_name[info.func_id],
return_value,
info.loc);
bpftrace->out_->helper_error(info.func_id, return_value, info.loc);
return;
}
else if (printf_id == asyncactionint(AsyncAction::watchpoint_attach))
Expand Down
55 changes: 43 additions & 12 deletions src/output.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@
#include "utils.h"
#include <async_event_types.h>

#include <bpf/libbpf.h>

namespace libbpf {
#define __BPF_NAME_FN(x) #x
const char *bpf_func_name[] = { __BPF_FUNC_MAPPER(__BPF_NAME_FN) };
#undef __BPF_NAME_FN
} // namespace libbpf

namespace bpftrace {

namespace {
Expand Down Expand Up @@ -147,6 +155,30 @@ void Output::lhist_prepare(const std::vector<uint64_t> &values, int min, int max
}
}

std::string Output::get_helper_error_msg(int func_id, int retcode) const
{
std::string msg;
if (func_id == libbpf::BPF_FUNC_map_update_elem && retcode == -E2BIG)
{
msg = "Map full; can't update element. Try increasing MAP_KEYS_MAX.";
Copy link
Member

Choose a reason for hiding this comment

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

This is the "old" name for w/ the config changes. Sounds like there's consensus on changing the env var names - should we preemptively use BPFTRACE_MAX_MAP_KEYS here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Depends on the merging order :) This is a smaller change so I imagine this will get merged first. Happy to change it with the config PR.

Copy link
Member

Choose a reason for hiding this comment

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

I think this is good to merge. Then the other PR can rebase on top

}
else if (func_id == libbpf::BPF_FUNC_map_delete_elem && retcode == -ENOENT)
{
msg = "Can't delete map element because it does not exist.";
}
// bpftrace sets the return code to 0 for map_lookup_elem failures
// which is why we're not also checking the retcode
else if (func_id == libbpf::BPF_FUNC_map_lookup_elem)
{
msg = "Can't lookup map element because it does not exist.";
}
else
{
msg = strerror(-retcode);
}
return msg;
}

std::string Output::value_to_str(BPFtrace &bpftrace,
const SizedType &type,
std::vector<uint8_t> &value,
Expand Down Expand Up @@ -565,17 +597,14 @@ void TextOutput::attached_probes(uint64_t num_probes) const
out_ << "Attaching " << num_probes << " probes..." << std::endl;
}

void TextOutput::helper_error(const std::string &helper,
void TextOutput::helper_error(int func_id,
int retcode,
const location &loc) const
{
std::stringstream msg;
msg << "Failed to " << helper << ": ";
if (retcode < 0)
msg << strerror(-retcode) << " (" << retcode << ")";
else
msg << retcode;
LOG(WARNING, loc, out_) << msg.str();
LOG(WARNING, loc, out_) << get_helper_error_msg(func_id, retcode)
<< "\nAdditional Info - helper: "
<< libbpf::bpf_func_name[func_id]
<< ", retcode: " << retcode;
}

std::string TextOutput::field_to_str(const std::string &name,
Expand Down Expand Up @@ -855,13 +884,15 @@ void JsonOutput::attached_probes(uint64_t num_probes) const
message(MessageType::attached_probes, "probes", num_probes);
}

void JsonOutput::helper_error(const std::string &helper,
void JsonOutput::helper_error(int func_id,
int retcode,
const location &loc) const
{
out_ << "{\"type\": \"helper_error\", \"helper\": \"" << helper
<< "\", \"retcode\": " << retcode << ", \"line\": " << loc.begin.line
<< ", \"col\": " << loc.begin.column << "}" << std::endl;
out_ << "{\"type\": \"helper_error\", \"msg\": \""
<< get_helper_error_msg(func_id, retcode) << "\", \"helper\": \""
<< libbpf::bpf_func_name[func_id] << "\", \"retcode\": " << retcode
<< ", \"line\": " << loc.begin.line << ", \"col\": " << loc.begin.column
<< "}" << std::endl;
}

std::string JsonOutput::field_to_str(const std::string &name,
Expand Down
7 changes: 4 additions & 3 deletions src/output.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class Output
virtual void message(MessageType type, const std::string& msg, bool nl = true) const = 0;
virtual void lost_events(uint64_t lost) const = 0;
virtual void attached_probes(uint64_t num_probes) const = 0;
virtual void helper_error(const std::string &helper,
virtual void helper_error(int func_id,
int retcode,
const location &loc) const = 0;

Expand All @@ -86,6 +86,7 @@ class Output
std::ostream &err_;
void hist_prepare(const std::vector<uint64_t> &values, int &min_index, int &max_index, int &max_value) const;
void lhist_prepare(const std::vector<uint64_t> &values, int min, int max, int step, int &max_index, int &max_value, int &buckets, int &start_value, int &end_value) const;
std::string get_helper_error_msg(int func_id, int retcode) const;
// Convert a log2 histogram into string
virtual std::string hist_to_str(const std::vector<uint64_t> &values,
uint32_t div,
Expand Down Expand Up @@ -194,7 +195,7 @@ class TextOutput : public Output {
void message(MessageType type, const std::string& msg, bool nl = true) const override;
void lost_events(uint64_t lost) const override;
void attached_probes(uint64_t num_probes) const override;
void helper_error(const std::string &helper,
void helper_error(int func_id,
int retcode,
const location &loc) const override;

Expand Down Expand Up @@ -249,7 +250,7 @@ class JsonOutput : public Output {
void message(MessageType type, const std::string& field, uint64_t value) const;
void lost_events(uint64_t lost) const override;
void attached_probes(uint64_t num_probes) const override;
void helper_error(const std::string &helper,
void helper_error(int func_id,
int retcode,
const location &loc) const override;

Expand Down
2 changes: 1 addition & 1 deletion tests/runtime/json-output
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ TIMEOUT 1

NAME helper_error
RUN {{BPFTRACE}} -kk -q -f json -e 'struct foo {int a;}; BEGIN { $tmp = ((struct foo*) 0)->a; exit(); }'
EXPECT {"type": "helper_error", "helper": "probe_read", "retcode": -14, "line": 1, "col": 37}
EXPECT {"type": "helper_error", "msg": "Bad address", "helper": "probe_read", "retcode": -14, "line": 1, "col": 37}
TIMEOUT 1

NAME cgroup_path
Expand Down
4 changes: 2 additions & 2 deletions tests/runtime/other
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,10 +263,10 @@ TIMEOUT 1

NAME runtime_error_check_delete
RUN {{BPFTRACE}} -k -e 'i:ms:100 { @[1] = 1; delete(@[2]); exit(); }'
EXPECT WARNING: Failed to map_delete_elem: No such file or directory \(-2\)
EXPECT WARNING: Can't delete map element because it does not exist.
TIMEOUT 1

NAME runtime_error_check_lookup
RUN {{BPFTRACE}} -kk -e 'i:ms:100 { @[1] = 1; printf("%d\n", @[2]); exit(); }'
EXPECT WARNING: Failed to map_lookup_elem: 0
EXPECT WARNING: Can't lookup map element because it does not exist.
TIMEOUT 1