Skip to content

Commit

Permalink
usdt: fix parsing sp register in arguments on AArch64
Browse files Browse the repository at this point in the history
One of the USDT probes for OpenJDK on AArch64 has an argument as an
offset from the stack pointer register like "8@[sp, 112]". This causes
the argument parser to fail:

  Parse error:
      8@x22 8@x20 8@x23 8@x0 8@x26 8@x27 8@[sp, 112] 8@[sp, 120]
  ------------------------------------------^

The error message then repeats forever.

Changed ArgumentParser_aarch64::parse_register so it accepts either
"xNN" or "sp" and outputs the register name rather than the register
number. The stack pointer is in a separate field `sp` in `struct
pt_regs` rather than in the `regs[]` array [1].

Note that the parser currently accepts "x31" and converts that into a
reference to `regs[31]' but that array only has 31 elements. Made x31 an
alias for `sp` to avoid undefined behaviour from reading past the end of
the array.

[1]: https://elixir.bootlin.com/linux/latest/source/arch/arm64/include/asm/ptrace.h#L160

Change-Id: I88b6ff741914b5d06ad5798a55bd21ea03f69825
Signed-off-by: Nick Gasson <nick.gasson@arm.com>
  • Loading branch information
nick-arm authored and yonghong-song committed Mar 8, 2020
1 parent aeea0e9 commit 5011f99
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 19 deletions.
6 changes: 4 additions & 2 deletions src/cc/usdt.h
Expand Up @@ -102,6 +102,8 @@ class ArgumentParser {
}
bool error_return(ssize_t error_start, ssize_t skip_start) {
print_error(error_start);
if (isspace(arg_[skip_start]))
skip_start++; // Make sure we skip at least one character
skip_until_whitespace_from(skip_start);
return false;
}
Expand All @@ -115,9 +117,9 @@ class ArgumentParser {

class ArgumentParser_aarch64 : public ArgumentParser {
private:
bool parse_register(ssize_t pos, ssize_t &new_pos, optional<int> *reg_num);
bool parse_register(ssize_t pos, ssize_t &new_pos, std::string &reg_name);
bool parse_size(ssize_t pos, ssize_t &new_pos, optional<int> *arg_size);
bool parse_mem(ssize_t pos, ssize_t &new_pos, optional<int> *reg_num,
bool parse_mem(ssize_t pos, ssize_t &new_pos, std::string &reg_name,
optional<int> *offset);

public:
Expand Down
46 changes: 31 additions & 15 deletions src/cc/usdt/usdt_args.cc
Expand Up @@ -133,11 +133,27 @@ void ArgumentParser::skip_until_whitespace_from(size_t pos) {
}

bool ArgumentParser_aarch64::parse_register(ssize_t pos, ssize_t &new_pos,
optional<int> *reg_num) {
new_pos = parse_number(pos, reg_num);
if (new_pos == pos || *reg_num < 0 || *reg_num > 31)
std::string &reg_name) {
if (arg_[pos] == 'x') {
optional<int> reg_num;
new_pos = parse_number(pos + 1, &reg_num);
if (new_pos == pos + 1 || *reg_num < 0 || *reg_num > 31)
return error_return(pos + 1, pos + 1);

if (*reg_num == 31) {
reg_name = "sp";
} else {
reg_name = "regs[" + std::to_string(reg_num.value()) + "]";
}

return true;
} else if (arg_[pos] == 's' && arg_[pos + 1] == 'p') {
reg_name = "sp";
new_pos = pos + 2;
return true;
} else {
return error_return(pos, pos);
return true;
}
}

bool ArgumentParser_aarch64::parse_size(ssize_t pos, ssize_t &new_pos,
Expand All @@ -156,11 +172,9 @@ bool ArgumentParser_aarch64::parse_size(ssize_t pos, ssize_t &new_pos,
}

bool ArgumentParser_aarch64::parse_mem(ssize_t pos, ssize_t &new_pos,
optional<int> *reg_num,
std::string &reg_name,
optional<int> *offset) {
if (arg_[pos] != 'x')
return error_return(pos, pos);
if (parse_register(pos + 1, new_pos, reg_num) == false)
if (parse_register(pos, new_pos, reg_name) == false)
return false;

if (arg_[new_pos] == ',') {
Expand Down Expand Up @@ -195,20 +209,22 @@ bool ArgumentParser_aarch64::parse(Argument *dest) {
return error_return(new_pos, new_pos);
cur_pos = new_pos + 1;

if (arg_[cur_pos] == 'x') {
if (arg_[cur_pos] == 'x' || arg_[cur_pos] == 's') {
// Parse ...@<reg>
optional<int> reg_num;
if (parse_register(cur_pos + 1, new_pos, &reg_num) == false)
std::string reg_name;
if (parse_register(cur_pos, new_pos, reg_name) == false)
return false;

cur_pos_ = new_pos;
dest->base_register_name_ = "regs[" + std::to_string(reg_num.value()) + "]";
dest->base_register_name_ = reg_name;
} else if (arg_[cur_pos] == '[') {
// Parse ...@[<reg>] and ...@[<reg,<offset>]
optional<int> reg_num, offset = 0;
if (parse_mem(cur_pos + 1, new_pos, &reg_num, &offset) == false)
optional<int> offset = 0;
std::string reg_name;
if (parse_mem(cur_pos + 1, new_pos, reg_name, &offset) == false)
return false;
cur_pos_ = new_pos;
dest->base_register_name_ = "regs[" + std::to_string(reg_num.value()) + "]";
dest->base_register_name_ = reg_name;
dest->deref_offset_ = offset;
} else {
// Parse ...@<value>
Expand Down
7 changes: 5 additions & 2 deletions tests/cc/test_usdt_args.cc
Expand Up @@ -74,11 +74,14 @@ TEST_CASE("test usdt argument parsing", "[usdt]") {
}
SECTION("argument examples from the Python implementation") {
#ifdef __aarch64__
USDT::ArgumentParser_aarch64 parser("-1@x0 4@5 8@[x12] -4@[x31,-40]");
USDT::ArgumentParser_aarch64 parser(
"-1@x0 4@5 8@[x12] -4@[x30,-40] -4@[x31,-40] 8@[sp, 120]");
verify_register(parser, -1, "regs[0]");
verify_register(parser, 4, 5);
verify_register(parser, 8, "regs[12]", 0);
verify_register(parser, -4, "regs[31]", -40);
verify_register(parser, -4, "regs[30]", -40);
verify_register(parser, -4, "sp", -40);
verify_register(parser, 8, "sp", 120);
#elif __powerpc64__
USDT::ArgumentParser_powerpc64 parser(
"-4@0 8@%r0 8@i0 4@0(%r0) -2@0(0) "
Expand Down

0 comments on commit 5011f99

Please sign in to comment.