Skip to content
This repository has been archived by the owner on May 14, 2024. It is now read-only.

Commit

Permalink
Fix bugs with name expression map APIs and test
Browse files Browse the repository at this point in the history
For the API, switch to using section names instead of flags to identify
specific sections. It appears the flags set may differ across machines.

For the test, correctly set up nameExpressions and symbolNames to have
two different name expressions, one for each example in the test source
code. This fixes an invalid memory read that causes errors on some systems.
Also fix map fetch to correctly check code object instead of bitcode in second
part of test.

Change-Id: I4217f616995635515c637d38fac40303225ee6f7
  • Loading branch information
lamb-j committed Oct 3, 2023
1 parent a4cd0c7 commit e13ccf3
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 11 deletions.
32 changes: 23 additions & 9 deletions lib/comgr/src/comgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2005,17 +2005,31 @@ amd_comgr_populate_name_expression_map(amd_comgr_data_t Data,

Elf_Shdr_Impl<ELF64LE> dynsymShdr, relaShdr, rodataShdr;
for (auto Shdr : Sections) {
if (Shdr.sh_type == llvm::ELF::SHT_DYNSYM)

if (Shdr.sh_type == ELF::SHT_DYNSYM)
dynsymShdr = Shdr;

if (Shdr.sh_type == ELF::SHT_RELA)
// Check sh_info to differentiate .rela.dyn and not .rela
if (Shdr.sh_type == ELF::SHT_RELA && Shdr.sh_info == 0)
relaShdr = Shdr;

if ((Shdr.sh_type == ELF::SHT_PROGBITS) &&
!(Shdr.sh_flags & ELF::SHF_WRITE) &&
!(Shdr.sh_flags & ELF::SHF_EXECINSTR) &&
!(Shdr.sh_flags & ELF::SHF_MASKPROC) )
rodataShdr = Shdr;
// We can't uniquely identify the .rodata section using the type and flag
// because other sections may use the exact same flags and type (i.e.
// .interp). For correctness, we can check the name instead
if (Shdr.sh_type == ELF::SHT_PROGBITS &&
(Shdr.sh_flags & ELF::SHF_ALLOC)) {

Expected<StringRef> SecNameOrError = ELFFile.getSectionName(Shdr);
if (!SecNameOrError) {
llvm::logAllUnhandledErrors(SecNameOrError.takeError(),
llvm::errs(), "ELFObj creation error: ");
return AMD_COMGR_STATUS_ERROR;
}
StringRef SecName = std::move(SecNameOrError.get());

if (SecName.equals(StringRef(".rodata")))
rodataShdr = Shdr;
}
}

// .dynsym - Find name expressions with amdgcn_name_expr and store their
Expand Down Expand Up @@ -2097,9 +2111,9 @@ amd_comgr_populate_name_expression_map(amd_comgr_data_t Data,

// Collect an unmangled name for each name expression
for (auto expData : nameExpDataVec) {
// TODO: If/when an accessor API becomes availble to get the starting
// TODO: If/when an accessor API becomes available to get the starting
// address for the section, switch to that
int offset = expData->RodataOffset - rodataShdr.sh_offset;
size_t offset = expData->RodataOffset - rodataShdr.sh_offset;

// Store from the offset up until the first '\0'
const char *unmangled =
Expand Down
6 changes: 4 additions & 2 deletions lib/comgr/test/name_expression_map_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,10 @@ int main(int argc, char *argv[]) {
exit(1);
}

char *nameExpressions[] = {"my_kernel_BOO<static_cast<int>(2+1),float >"};
char *symbolNames[] = {"_Z13my_kernel_BOOILi3EfEvPT0_"};
char *nameExpressions[] = {"my_kernel_BOO<static_cast<int>(2+1),float >",
"my_kernel_FOO<static_cast<int>(2+1),float >"};
char *symbolNames[] = {"_Z13my_kernel_BOOILi3EfEvPT0_",
"_Z13my_kernel_FOOILi3EfEvPT0_"};

for (size_t I = 0; I < numNames; ++I) {
size_t Size;
Expand Down

0 comments on commit e13ccf3

Please sign in to comment.