Skip to content

Commit

Permalink
DwarfWalker: clean up interfaces for findDieName and findName (#1154)
Browse files Browse the repository at this point in the history 
findDieName always returned 'true', so any checks for its return value
were meaningless. It was also not clear that 'findName' would modify its
argument. The new interface removes that surprise by making the name the
return value.

Co-authored-by: Tim Haines <thaines@cs.wisc.edu>
  • Loading branch information
@hainest
hainest and Tim Haines committed Nov 18, 2021
1 parent 4f6f1a2 commit 71027cb
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 50 deletions.
11 changes: 3 additions & 8 deletions symtabAPI/src/Object-elf.C
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2243,10 +2243,7 @@ bool Object::dwarf_parse_aranges(Dwarf *dbg, std::set<Dwarf_Off> &/*dies_seen*/)
auto off_die = dwarf_dieoffset(&cu_die);
//if (dies_seen.find(off_die) != dies_seen.end()) continue;

std::string modname;
if (!DwarfWalker::findDieName(cu_die, modname)) {
modname = associated_symtab->file(); // default module
}
std::string modname = DwarfWalker::die_name(cu_die);

Offset actual_start, actual_end;
convertDebugOffset(start, actual_start);
Expand Down Expand Up @@ -2301,10 +2298,8 @@ bool Object::fix_global_symbol_modules_static_dwarf() {
for (size_t i = 0; i < dies.size(); i++) {
Dwarf_Die cu_die = dies[i];

std::string modname;
if (!DwarfWalker::findDieName(cu_die, modname)) {
modname = associated_symtab->file(); // default module
}
std::string modname = DwarfWalker::die_name(cu_die);

if(modname=="<artificial>")
{
auto off_die = dwarf_dieoffset(&cu_die);
Expand Down
68 changes: 28 additions & 40 deletions symtabAPI/src/dwarfWalker.C
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,7 @@ bool DwarfWalker::parseModule(Dwarf_Die moduleDIE, Module *&fixUnknownMod) {
return false;

/* Extract the name of this module. */
std::string moduleName;
if (!findDieName(moduleDIE, moduleName)) return false;
std::string moduleName = die_name(moduleDIE);

// DIEs without name or named <artificial> will be associated to
// the default module (whose name is ELF filename)
Expand Down Expand Up @@ -819,8 +818,7 @@ bool DwarfWalker::parseLexicalBlock() {
bool DwarfWalker::parseCommonBlock() {
dwarf_printf("(0x%lx) Parsing common block\n", id());

std::string commonBlockName;
if (!findDieName(entry(), commonBlockName)) return false;
std::string commonBlockName = die_name(entry());
Symbol* commonBlockVar = findSymbolForCommonBlock(commonBlockName);
if(!commonBlockVar)
{
Expand Down Expand Up @@ -878,9 +876,7 @@ bool DwarfWalker::parseVariable() {
if (!handleSpecification(hasSpecification))
return false;

if (!findName(curName()))
return false;

curName() = std::move(die_name());
removeFortranUnderscore(curName());

/* We'll start with the location, since that's most likely to
Expand Down Expand Up @@ -1040,7 +1036,7 @@ bool DwarfWalker::parseFormalParam() {
return false;
}

if (!findName(curName())) return false;
curName() = std::move(die_name());
/* We can't do anything with anonymous parameters. */
if (!nameDefined()) {
dwarf_printf("(0x%lx) No name associated with formal, returning\n", id());
Expand Down Expand Up @@ -1085,7 +1081,7 @@ bool DwarfWalker::parseBaseType() {
if(!tc()) return false;
dwarf_printf("(0x%lx) parseBaseType entry\n", id());

if (!findName(curName())) return false;
curName() = std::move(die_name());
if (!nameDefined()) {
dwarf_printf("(0x%lx) No name for type, returning early\n", id());
return true;
Expand Down Expand Up @@ -1169,7 +1165,7 @@ bool DwarfWalker::parseTypedef() {
boost::shared_ptr<Type> referencedType;
if (!findType(referencedType, true)) return false;

if (!findName(curName())) return false;
curName() = std::move(die_name());
if (!nameDefined()) {
if (!fixName(curName(), referencedType)) return false;
}
Expand Down Expand Up @@ -1201,7 +1197,7 @@ bool DwarfWalker::parseArray() {
if (!findType(elementType, false)) return false;
if (!elementType) return false;

if (!findName(curName())) return false;
curName() = std::move(die_name());

// curName may get overridden by the subrange parsing code.
// TODO: make this part of the context stack.
Expand Down Expand Up @@ -1262,7 +1258,7 @@ bool DwarfWalker::parseSubrange() {
bool DwarfWalker::parseEnum() {
if(!tc()) return false;
dwarf_printf("(0x%lx) parseEnum entry\n", id());
if (!findName(curName())) return false;
curName() = std::move(die_name());
//setEnum(tc()->addOrUpdateType( Type::make_shared<typeEnum>( type_id(), "enum " + curName())));
setEnum(tc()->addOrUpdateType( Type::make_shared<typeEnum>( type_id(), curName())));
return true;
Expand Down Expand Up @@ -1301,7 +1297,7 @@ bool DwarfWalker::parseStructUnionClass() {
return false;
}

if (!findName(curName())) return false;
curName() = std::move(die_name());
dwarf_printf("(0x%lx) Struct/Union/Class name from dwarf: %s\n", id(), curName().c_str());

if (!nameDefined()) {
Expand Down Expand Up @@ -1372,8 +1368,7 @@ bool DwarfWalker::parseStructUnionClass() {
bool DwarfWalker::parseEnumEntry() {
dwarf_printf("(0x%lx) parseEnumEntry entry\n", id());

std::string name;
if (!findName(name)) return false;
std::string name = die_name();

long value = 0;
bool valid;
Expand All @@ -1391,7 +1386,7 @@ bool DwarfWalker::parseMember() {
if (!findType(memberType, false)) return false;
if (!memberType) return false;

if (!findName(curName())) return false;
curName() = std::move(die_name());

long value;
bool hasValue;
Expand Down Expand Up @@ -1447,7 +1442,7 @@ bool DwarfWalker::parseConstPackedVolatile() {
boost::shared_ptr<Type> type = NULL;
if (!findType(type, true)) return false;

if (!findName(curName())) return false;
curName() = std::move(die_name());
if (!nameDefined()) {
dwarf_printf("(0x%lx) parseConstPackedVolatile fixName\n", id());
if (!fixName(curName(), type)) return false;
Expand All @@ -1467,11 +1462,7 @@ bool DwarfWalker::parseTypeReferences() {
return false;
}

if (!findName(curName()))
{
dwarf_printf("(0x%lx) name not found\n", id());
return false;
}
curName() = std::move(die_name());

Dwarf_Die e = entry();
boost::shared_ptr<Type> indirectType;
Expand Down Expand Up @@ -1576,22 +1567,21 @@ bool DwarfWalker::handleSpecification(bool &hasSpec) {
return true;
}

bool DwarfWalker::findDieName(Dwarf_Die die, std::string &name)
std::string DwarfWalker::die_name(Dwarf_Die die)
{
auto cname = dwarf_diename(&die);
if (cname == 0) {
name = std::string();
return true;
}
auto name = dwarf_diename(&die);

name = cname;
return true;
// You cannot construct a std::string from a null pointer
if (name) {
return name;
}
return {};
}

bool DwarfWalker::findName(std::string &name) {
if (!findDieName(specEntry(), name)) return false;
std::string DwarfWalker::die_name() {
auto name = die_name(specEntry());
dwarf_printf("(0x%lx) Found name %s.\n", id(), name.c_str());
return true;
return name;
}


Expand All @@ -1617,7 +1607,8 @@ bool DwarfWalker::findFuncName() {
dwarf_printf("(0x%lx) DW_AT_linkage_name name not found\n", id());

setMangledName(false);
return findDieName(entry(), curName());
curName() = std::move(die_name(entry()));
return true;
}

std::vector<VariableLocation>& DwarfParseActions::getFramePtrRefForInit()
Expand Down Expand Up @@ -2265,10 +2256,7 @@ bool DwarfWalker::parseSubrangeAUX(Dwarf_Die entry,
} /* end if we found an upper bound or count. */

/* Construct the range type. */
if (!findName(curName())) {
dwarf_printf("cannot find subrange name %s\n", curName().c_str());
return false;
}
curName() = std::move(die_name());
if (!nameDefined()) {
curName() = "{anonymousRange}";
}
Expand Down Expand Up @@ -2403,8 +2391,8 @@ bool DwarfWalker::decipherBound(Dwarf_Attribute boundAttribute, bool /*is_info*/
if(!ret_p) return false;

/* Does it have a name? */
if (findDieName(boundEntry, boundString)
&& !boundString.empty())
boundString = std::move(die_name(boundEntry));
if (!boundString.empty())
return true;

/* Does it describe a nameless constant? */
Expand Down
4 changes: 2 additions & 2 deletions symtabAPI/src/dwarfWalker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -344,9 +344,9 @@ class DwarfWalker : public DwarfParseActions {
bool &hasLineNumber,
std::string &filename);
public:
static bool findDieName(Dwarf_Die die, std::string &);
static std::string die_name(Dwarf_Die die);
private:
bool findName(std::string &);
std::string die_name();
void removeFortranUnderscore(std::string &);
bool findSize(unsigned &size);
bool findVisibility(visibility_t &visibility);
Expand Down

0 comments on commit 71027cb

Please sign in to comment.