Skip to content

Commit 5bc6b43

Browse files
committed
Capture TPIDR register on Aarch64 and report it to the debugger.
Ubuntu 26.04 LTS has gdb 17.1. This causes test failures in the `tls` test; this version of gdb seems to expect a `org.gnu.gdb.aarch64.tls` feature in the registers list, containing a `tpidr` register. Supporting that is a bit of a pain. As far as I can tell, we have no choice but to store it in `ExtraRegisters`, which means extending the format of that data, and the register won't be present in older traces.
1 parent c72d339 commit 5bc6b43

10 files changed

Lines changed: 85 additions & 21 deletions

src/ExtraRegisters.cc

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ struct RegData {
5454

5555
RegData(int offset = -1, int size = 0)
5656
: offset(offset), size(size), xsave_feature_bit(-1) {}
57+
size_t end() const {
58+
return offset + size;
59+
}
5760
};
5861

5962
static bool reg_in_range(GdbServerRegister regno, GdbServerRegister low, GdbServerRegister high,
@@ -259,7 +262,7 @@ static uint64_t* xsave_features(vector<uint8_t>& data) {
259262

260263
size_t ExtraRegisters::read_register(uint8_t* buf, GdbServerRegister regno,
261264
bool* defined) const {
262-
if (format_ == NT_FPR) {
265+
if (format_ == AARCH64_FPR) {
263266
if (arch() != aarch64) {
264267
*defined = false;
265268
return 0;
@@ -290,12 +293,19 @@ size_t ExtraRegisters::read_register(uint8_t* buf, GdbServerRegister regno,
290293
memcpy(buf, &mask, sizeof(mask));
291294
return sizeof(mask);
292295
#endif
296+
} else if (regno == DREG_TPIDR) {
297+
reg_data = RegData(sizeof(ARM64Arch::user_fpsimd_state),
298+
sizeof(uint64_t));
299+
if (reg_data.end() > data_.size()) {
300+
*defined = false;
301+
return 0;
302+
}
293303
} else {
294304
*defined = false;
295305
return 0;
296306
}
297307

298-
DEBUG_ASSERT(size_t(reg_data.offset + reg_data.size) <= data_.size());
308+
DEBUG_ASSERT(reg_data.end() <= data_.size());
299309
*defined = true;
300310
memcpy(buf, data_.data() + reg_data.offset, reg_data.size);
301311
return reg_data.size;
@@ -332,7 +342,7 @@ size_t ExtraRegisters::read_register(uint8_t* buf, GdbServerRegister regno,
332342

333343
bool ExtraRegisters::write_register(GdbServerRegister regno, const void* value,
334344
size_t value_size) {
335-
if (format_ == NT_FPR) {
345+
if (format_ == AARCH64_FPR) {
336346
if (arch() != aarch64) {
337347
return false;
338348
}
@@ -538,7 +548,7 @@ void ExtraRegisters::print_register_file_compact(FILE* f) const {
538548
print_regs(*this, DREG_64_XMM0, DREG_64_YMM0H, 16, "ymm", f);
539549
break;
540550
case aarch64:
541-
DEBUG_ASSERT(format_ == NT_FPR);
551+
DEBUG_ASSERT(format_ == AARCH64_FPR);
542552
print_regs(*this, DREG_V0, GdbServerRegister(0), 32, "v", f);
543553
fputc(' ', f);
544554
print_reg(*this, DREG_FPSR, GdbServerRegister(0), "fpsr", f);
@@ -629,11 +639,11 @@ bool ExtraRegisters::set_to_raw_data(SupportedArch a, Format format,
629639

630640
if (format == NONE) {
631641
return true;
632-
} else if (format == NT_FPR) {
642+
} else if (format == AARCH64_FPR) {
633643
if (!memcpy_fpr_regs_arch(a, data_, data, data_size)) {
634644
return false;
635645
}
636-
format_ = NT_FPR;
646+
format_ = AARCH64_FPR;
637647
return true;
638648
}
639649

@@ -788,7 +798,7 @@ vector<uint8_t> ExtraRegisters::get_user_fpregs_struct(
788798
return to_vector(
789799
*reinterpret_cast<const X64Arch::user_fpregs_struct*>(data_.data()));
790800
case aarch64:
791-
DEBUG_ASSERT(format_ == NT_FPR);
801+
DEBUG_ASSERT(format_ == AARCH64_FPR);
792802
DEBUG_ASSERT(data_.size() == sizeof(ARM64Arch::user_fpregs_struct));
793803
return to_vector(
794804
*reinterpret_cast<const ARM64Arch::user_fpregs_struct*>(data_.data()));
@@ -816,7 +826,7 @@ void ExtraRegisters::set_user_fpregs_struct(Task* t, SupportedArch arch,
816826
memcpy(data_.data(), data, sizeof(X64Arch::user_fpregs_struct));
817827
return;
818828
case aarch64:
819-
DEBUG_ASSERT(format_ == NT_FPR);
829+
DEBUG_ASSERT(format_ == AARCH64_FPR);
820830
ASSERT(t, size >= sizeof(ARM64Arch::user_fpregs_struct));
821831
ASSERT(t, data_.size() >= sizeof(ARM64Arch::user_fpregs_struct));
822832
memcpy(data_.data(), data, sizeof(ARM64Arch::user_fpregs_struct));
@@ -890,7 +900,7 @@ void ExtraRegisters::reset() {
890900
memcpy(data_.data() + xinuse_offset, &xinuse, sizeof(xinuse));
891901
}
892902
} else {
893-
DEBUG_ASSERT(format_ == NT_FPR);
903+
DEBUG_ASSERT(format_ == AARCH64_FPR);
894904
DEBUG_ASSERT(arch() == aarch64 &&
895905
"Ensure that nothing is required here for your architecture.");
896906
}
@@ -953,7 +963,7 @@ void ExtraRegisters::compare_internal(const ExtraRegisters& reg2,
953963
compare_regs(*this, reg2, DREG_64_XMM0, avx_present ? DREG_64_YMM0H : NOT_PRESENT, 8, "ymm", result);
954964
break;
955965
case aarch64:
956-
DEBUG_ASSERT(format_ == NT_FPR);
966+
DEBUG_ASSERT(format_ == AARCH64_FPR);
957967
compare_regs(*this, reg2, DREG_V0, NOT_PRESENT, 32, "v", result);
958968
break;
959969
default:

src/ExtraRegisters.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,12 @@ class ExtraRegisters {
5858
*/
5959
XSAVE,
6060
/**
61-
* Stores the content of the NT_FPREGS regset. The format depends on the
62-
* architecture. It is given by Arch::user_fpregs_struct for the appropriate
63-
* architecture.
61+
* Stores the content of the NT_FPREGS regset for Aarch64.
62+
* The first sizeof(ARM64Arch::user_fpregs_struct) bytes (or less) are
63+
* a ARM64Arch::user_fpregs_struct.
64+
* The next 8 bytes after that (if present) are TPIDR.
6465
*/
65-
NT_FPR };
66+
AARCH64_FPR };
6667

6768
// Set values from raw data, with the given XSAVE layout. Returns false
6869
// if this could not be done.

src/GdbServer.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2340,6 +2340,8 @@ const vector<GdbServerRegister>& GdbServer::target_registers(
23402340
cpu_features & (1 << static_cast<uint8_t>(TargetFeature::AVX512));
23412341
bool have_PAUTH =
23422342
cpu_features & (1 << static_cast<uint8_t>(TargetFeature::PAuth));
2343+
bool have_TPIDR =
2344+
cpu_features & (1 << static_cast<uint8_t>(TargetFeature::Tls));
23432345
switch (arch) {
23442346
case x86: {
23452347
add_range(GdbServerRegister(0), GdbServerRegister(DREG_ORIG_EAX));
@@ -2377,6 +2379,9 @@ const vector<GdbServerRegister>& GdbServer::target_registers(
23772379
case aarch64:
23782380
add_range(GdbServerRegister::DREG_X0,
23792381
GdbServerRegister::DREG_FPCR);
2382+
if (have_TPIDR) {
2383+
register_description.push_back(DREG_TPIDR);
2384+
}
23802385
if (have_PAUTH) {
23812386
register_description.push_back(DREG_PAUTH_DMASK);
23822387
register_description.push_back(DREG_PAUTH_CMASK);

src/GdbServerRegister.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,9 @@ enum GdbServerRegister {
320320
DREG_FPSR,
321321
DREG_FPCR,
322322

323+
// aarch64 TLS
324+
DREG_TPIDR,
325+
323326
// aarch64-pauth.xml
324327
DREG_PAUTH_DMASK,
325328
DREG_PAUTH_CMASK,

src/TargetDescription.cc

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,19 @@ FeatureStream& operator<<(FeatureStream& stream, rr::SupportedArch arch) {
5151
return stream;
5252
}
5353

54+
static const char* aarch64_tls_xml = R"(<feature name="org.gnu.gdb.aarch64.tls">
55+
<reg name="tpidr" bitsize="64" type="data_ptr"/>
56+
</feature>
57+
)";
58+
5459
template <>
5560
FeatureStream& operator<<(FeatureStream& stream, TargetFeature feature) {
61+
if (feature == TargetFeature::Tls) {
62+
DEBUG_ASSERT(!strcmp(stream.arch_prefix, "aarch64-"));
63+
stream << aarch64_tls_xml;
64+
return stream;
65+
}
66+
5667
DEBUG_ASSERT(stream.arch_prefix != nullptr &&
5768
"No architecture has been provided to description");
5869
stream << R"( <xi:include href=")" << stream.arch_prefix;
@@ -84,6 +95,8 @@ FeatureStream& operator<<(FeatureStream& stream, TargetFeature feature) {
8495
case TargetFeature::PAuth:
8596
stream << "pauth.xml";
8697
break;
98+
default:
99+
CLEAN_FATAL() << "Unsupported feature";
87100
}
88101
stream << R"("/>)" << '\n';
89102
return stream;
@@ -123,10 +136,17 @@ static void get_x86_cpu_features(const TraceReader* trace,
123136
static void get_arm_cpu_features(const TraceReader* trace,
124137
vector<TargetFeature>& target_features) {
125138
bool pauth;
139+
bool tpidr;
126140
if (trace != nullptr) {
127141
pauth = trace->aarch64_pauth();
142+
tpidr = trace->aarch64_tpidr();
128143
} else {
129144
pauth = aarch64_pauth_enabled();
145+
tpidr = true;
146+
}
147+
148+
if (tpidr) {
149+
target_features.push_back(TargetFeature::Tls);
130150
}
131151
if (pauth) {
132152
target_features.push_back(TargetFeature::PAuth);

src/TargetDescription.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ enum class TargetFeature : uint8_t {
2020
PKeys,
2121
FPU,
2222
PAuth,
23+
Tls,
2324
};
2425

2526
class TargetDescription {

src/Task.cc

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1236,14 +1236,21 @@ const ExtraRegisters* Task::extra_regs_fallible() {
12361236
#elif defined(__aarch64__)
12371237
LOG(debug) << " (refreshing extra-register cache using FPR)";
12381238

1239-
extra_registers.format_ = ExtraRegisters::NT_FPR;
1240-
extra_registers.data_.resize(sizeof(ARM64Arch::user_fpregs_struct));
1239+
extra_registers.format_ = ExtraRegisters::AARCH64_FPR;
1240+
extra_registers.data_.resize(sizeof(ARM64Arch::user_fpregs_struct)
1241+
+ sizeof(uint64_t));
12411242
struct iovec vec = { extra_registers.data_.data(),
1242-
extra_registers.data_.size() };
1243+
sizeof(ARM64Arch::user_fpregs_struct) };
12431244
if (fallible_ptrace(PTRACE_GETREGSET, NT_PRFPREG, &vec)) {
12441245
return nullptr;
12451246
}
1246-
extra_registers.data_.resize(vec.iov_len);
1247+
memset(extra_registers.data_.data() + vec.iov_len, 0,
1248+
extra_registers.data_.data() + extra_registers.data_.size());
1249+
vec = { extra_registers.data_.data() + sizeof(ARM64Arch::user_fpregs_struct),
1250+
sizeof(uint64_t) };
1251+
if (fallible_ptrace(PTRACE_GETREGSET, NT_ARM_TLS, &vec)) {
1252+
return nullptr;
1253+
}
12471254
#else
12481255
#error need to define new extra_regs support
12491256
#endif
@@ -1807,14 +1814,24 @@ void Task::set_extra_regs(const ExtraRegisters& regs) {
18071814
}
18081815
break;
18091816
}
1810-
case ExtraRegisters::NT_FPR: {
1817+
case ExtraRegisters::AARCH64_FPR: {
18111818
struct iovec vec = { extra_registers.data_.data(),
1812-
extra_registers.data_.size() };
1819+
sizeof(ARM64Arch::user_fpsimd_state) };
18131820
if (ptrace_if_stopped(PTRACE_SETREGSET, NT_PRFPREG, &vec)) {
18141821
/* If that failed, the task was killed and it should not matter what
18151822
we tried to set. But we will remember that our registers are dirty. */
18161823
extra_registers_known = true;
18171824
}
1825+
if (extra_registers.data_.size() >=
1826+
sizeof(ARM64Arch::user_fpsimd_state) + 8) {
1827+
vec = { extra_registers.data_.data() + sizeof(ARM64Arch::user_fpsimd_state),
1828+
8 };
1829+
if (ptrace_if_stopped(PTRACE_SETREGSET, NT_ARM_TLS, &vec)) {
1830+
/* If that failed, the task was killed and it should not matter what
1831+
we tried to set. But we will remember that our registers are dirty. */
1832+
extra_registers_known = true;
1833+
}
1834+
}
18181835
break;
18191836
}
18201837
default:

src/TraceStream.cc

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -612,7 +612,7 @@ TraceFrame TraceReader::read_frame(FrameTime skip_before) {
612612
fmt = ExtraRegisters::XSAVE;
613613
break;
614614
case aarch64:
615-
fmt = ExtraRegisters::NT_FPR;
615+
fmt = ExtraRegisters::AARCH64_FPR;
616616
break;
617617
}
618618
bool ok = ret.recorded_extra_regs.set_to_raw_data(
@@ -1475,6 +1475,7 @@ void TraceWriter::close(CloseStatus status, const TraceUuid* uuid) {
14751475
} else {
14761476
auto aarch64_data = header.initAarch64();
14771477
aarch64_data.setHasPAuth(aarch64_pauth_enabled());
1478+
aarch64_data.setHasTPIDR(true);
14781479
}
14791480

14801481
{
@@ -1711,6 +1712,7 @@ TraceReader::TraceReader(const string& dir)
17111712
} else {
17121713
auto aarch64_data = header.getAarch64();
17131714
aarch64_pauth_ = aarch64_data.getHasPAuth();
1715+
aarch64_tpidr_ = aarch64_data.getHasTPIDR();
17141716
}
17151717

17161718
switch (header.getChaosMode()) {
@@ -1784,6 +1786,7 @@ TraceReader::TraceReader(const TraceReader& other)
17841786
cpu_improperly_configured_known_ = other.cpu_improperly_configured_known_;
17851787
cpu_improperly_configured_ = other.cpu_improperly_configured_;
17861788
aarch64_pauth_ = other.aarch64_pauth_;
1789+
aarch64_tpidr_ = other.aarch64_tpidr_;
17871790
exclusion_range_ = other.exclusion_range_;
17881791
uname_ = other.uname_;
17891792
quirks_ = other.quirks_;

src/TraceStream.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -512,6 +512,7 @@ class TraceReader : public TraceStream {
512512
return cpu_improperly_configured_;
513513
}
514514
bool aarch64_pauth() const { return aarch64_pauth_; }
515+
bool aarch64_tpidr() const { return aarch64_tpidr_; }
515516

516517
enum TraceQuirks {
517518
// Whether the /proc/<pid>/mem calls were explicitly recorded in this trace
@@ -556,6 +557,7 @@ class TraceReader : public TraceStream {
556557
bool cpu_improperly_configured_known_;
557558
bool cpu_improperly_configured_;
558559
bool aarch64_pauth_;
560+
bool aarch64_tpidr_;
559561
uint32_t syscallbuf_fds_disabled_size_;
560562
uint32_t syscallbuf_hdr_size_;
561563
int required_forward_compatibility_version_;

src/rr_trace.capnp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,8 @@ struct Header {
109109
aarch64 :group {
110110
# True if the tracee had HWCAP_PACA
111111
hasPAuth @30 :Bool;
112+
# True if the tracee recorded TPIDR
113+
hasTPIDR @31 :Bool;
112114
}
113115
# These flags guard rr behavior differences that ensure old rr traces can
114116
# be successfully replayed on newer replayers

0 commit comments

Comments
 (0)