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.

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

Update DwarfWalker subrange handling #1369

Merged
merged 33 commits into from
Feb 27, 2023
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
c907634
Remove redundant include of libdw.h
hainest Feb 3, 2023
5b6d213
Add subrange parsing to dynDwarf
hainest Feb 9, 2023
cf0591f
Remove decipherBound
hainest Feb 9, 2023
a2fcdda
Remove legacy comments and commented-out code from parseArray
hainest Feb 9, 2023
12a112f
Improve error message in parseArray
hainest Feb 9, 2023
47f4e74
Rename parseSubrangeAUX to parseSubrange and change interface
hainest Feb 9, 2023
7f86b0d
Replace bound calculations in parseSubrange
hainest Feb 9, 2023
bad087d
Update some comments and whitespace
hainest Feb 9, 2023
d8cf642
Update construction of the result type
hainest Feb 9, 2023
29636f0
Update debugging messages in parseSubrange
hainest Feb 9, 2023
0c0a3fa
Have parseSubrange() call parseSubrange(Dwarf_Die*)
hainest Feb 9, 2023
0ee23bc
Use updated parseSubrange in parseMultidimensionalArray
hainest Feb 9, 2023
108a1db
Use std::tostring instead of snprintf
hainest Feb 9, 2023
56d9e45
Update comments and remove dead code
hainest Feb 9, 2023
24f7efd
Whitespace
hainest Feb 9, 2023
c0bbe2c
parseMDA returns a typeArray instead of just a Type
hainest Feb 9, 2023
6407563
Fix bug in dwarf_result::operator bool
hainest Feb 9, 2023
0e610b7
Fix hi/low bound mixup in parseMDA
hainest Feb 9, 2023
7555de1
Try using Dyninst's parsed CU language, if needed
hainest Feb 9, 2023
3f4b3bf
Display subrange DIE ID in pareSubrange
hainest Feb 14, 2023
5725dd0
Don't create subrange type in parseSubrange
hainest Feb 14, 2023
4a9dd6e
Update parseSubrange() to use new parseSubrange(Dwarf_DIE*)
hainest Feb 14, 2023
79eb686
More carefully parse the child DIE
hainest Feb 14, 2023
51a2324
Iteratively parse the subranges in parseArray
hainest Feb 14, 2023
efa6f67
Whitespace
hainest Feb 14, 2023
da126ec
Remove parseMultiDimensionalArray
hainest Feb 15, 2023
c9e8852
Improve debug messages
hainest Feb 16, 2023
ea0e6a9
Register each subrange type in parseArray
hainest Feb 16, 2023
c33b43d
Remove common/h/dyninstversion.h
hainest Feb 21, 2023
344c357
Add copyright notices to new files
hainest Feb 21, 2023
7991ee6
Make dwarf_result also hold a Dwarf_Word
hainest Feb 23, 2023
aa9fb66
Only offer an interface to parse both bounds simultaneously
hainest Feb 23, 2023
1ad6566
Use new dwarf_subrange interface in DwarfWalker::parseSubrange
hainest Feb 23, 2023
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
10 changes: 10 additions & 0 deletions common/h/dyninstversion.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@

hainest marked this conversation as resolved.
Show resolved Hide resolved
#if !defined(DYNINST_VERSION_H)
#define DYNINST_VERSION_H

#define DYNINST_MAJOR_VERSION 12
#define DYNINST_MINOR_VERSION 2
#define DYNINST_PATCH_VERSION 1


#endif // DYNINST_VERSION_H
2 changes: 1 addition & 1 deletion dwarf/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ include_directories(src h ${PROJECT_SOURCE_DIR}/elf/h ${PROJECT_SOURCE_DIR}/comm
add_definitions(-DDYNDWARF_LIB)

set(SRC_LIST src/dwarfResult.C src/dwarfExprParser.C src/dwarfFrameParser.C
src/dwarfHandle.C)
src/dwarfHandle.C src/dwarf_subrange.cpp)

dyninst_library(dynDwarf dynElf common ${LibDwarf_LIBRARIES})
add_dependencies(dynDwarf ElfUtils)
Expand Down
115 changes: 115 additions & 0 deletions dwarf/src/dwarf_subrange.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
#include "dwarf_subrange.h"
hainest marked this conversation as resolved.
Show resolved Hide resolved

namespace {

Dwarf_Die *get_type(Dwarf_Die *die) {
Dwarf_Attribute scratch_attr;
dwarf_attr_integrate(die, DW_AT_type, &scratch_attr);
Dwarf_Die scratch_die;
Dwarf_Die *type = dwarf_formref_die(&scratch_attr, &scratch_die);

if (!type || dwarf_peel_type(type, type) != 0)
return nullptr;

return type;
}

bool is_signed(Dwarf_Die *die) {
Dwarf_Attribute attr;
if (dwarf_attr(get_type(die), DW_AT_encoding, &attr)) {
Dwarf_Word encoding;
if (dwarf_formudata(&attr, &encoding) == 0)
return encoding == DW_ATE_signed || encoding == DW_ATE_signed_char;
}
return false;
}
} // namespace

namespace Dyninst {
namespace DwarfDyninst {

dwarf_result<long> dwarf_subrange_upper_bound(Dwarf_Die *die) {
Copy link
Contributor

Choose a reason for hiding this comment

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

int64_t or Dwarf_Word instead of long? OK on linux and everywhere Dyninst runs, but long is required to be only at least 32-bits and would narrow the result if you were on a non-64-bit long system such as win64.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about C VLAs and C flexible arrays? The DWARF form type is an exprloc and not a constant integer type. I also see subranges with a type, but without a DW_AT_upper_bound attribute (it also changes an array of array to a single subrange without the DW_AT_upper_bound.

These could be deferred to a new issue: array types need an unknown (and maybe locexpr) upper bound value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switched using Dwarf_Word. VLAs and flexible arrays aren't handled in libdw's dwarf_aggregate_size. Maybe we can just ignore them here, as well?

Dwarf_Attribute attr;

/* This has either DW_AT_count or DW_AT_upper_bound. */
if (dwarf_attr_integrate(die, DW_AT_count, &attr)) {
Dwarf_Word count;
if (dwarf_formudata(&attr, &count) != 0)
return dwarf_error{};
return count;
Copy link
Contributor

Choose a reason for hiding this comment

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

return count + lower_bound - 1?

Works for zero-based array languages, but not 1-based arrays for languages or those that support a true range for the index values. Perhaps for efficiency there should only a function to return both lower and upper bounds as a pair so lower bound doesn't need to be computed twice if DW_AT_count is used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I refactored this with the added assumption of zero-based if there is DW_AT_count but no DW_AT_lower_bound or dwarf_srclang_lower_bound.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds reasonable

}

if (dwarf_attr_integrate(die, DW_AT_upper_bound, &attr)) {
if (is_signed(die)) {
Dwarf_Sword upper;
if (dwarf_formsdata(&attr, &upper) != 0)
return dwarf_error{};
return upper;
}
Dwarf_Word unsigned_upper;
if (dwarf_formudata(&attr, &unsigned_upper) != 0)
return dwarf_error{};
return unsigned_upper;
}

// Nothing was found, but there was no error
return dwarf_result<long>{};
}
dwarf_result<long> dwarf_subrange_lower_bound(Dwarf_Die *die) {
Dwarf_Attribute attr;

/* Having DW_AT_lower_bound is optional. */
if (dwarf_attr_integrate(die, DW_AT_lower_bound, &attr)) {
if (is_signed(die)) {
Dwarf_Sword lower;
if (dwarf_formsdata(&attr, &lower) != 0)
return dwarf_error{};
return lower;
}
Dwarf_Word unsigned_lower;
if (dwarf_formudata(&attr, &unsigned_lower) != 0)
return dwarf_error{};
return unsigned_lower;
}

// Try the default provided by the language
int lang = dwarf_srclang(die);
if (lang != -1) {
Dwarf_Sword lower;
if (dwarf_default_lower_bound(lang, &lower) != 0)
return dwarf_error{};
return lower;
}

// Nothing was found, but there was no error
// It's ok if we didn't find a srclang.
return dwarf_result<long>{};
}
dwarf_result<long> dwarf_subrange_length_from_enum(Dwarf_Die *die) {
/* We have to find the DW_TAG_enumerator child with the
highest value to know the array's element count. */
Dwarf_Die enum_child;
int has_children = dwarf_child(die, &enum_child);
if (has_children < 0)
return dwarf_error{};
if (has_children > 0) {
Dwarf_Attribute attr;
Dwarf_Word count{};
do {
if (dwarf_tag(&enum_child) == DW_TAG_enumerator) {
dwarf_attr_integrate(&enum_child, DW_AT_const_value, &attr);
Dwarf_Word value;
if (dwarf_formudata(&attr, &value) != 0)
return dwarf_error{};
if (value >= count)
count = value + 1;
}
} while (dwarf_siblingof(&enum_child, &enum_child) > 0);
return count;
}

// Nothing was found, but there was no error
return dwarf_result<long>{};
}
} // namespace DwarfDyninst
} // namespace Dyninst
26 changes: 26 additions & 0 deletions dwarf/src/dwarf_subrange.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#include "util.h"
hainest marked this conversation as resolved.
Show resolved Hide resolved
#include <boost/optional.hpp>
#include <dwarf.h>
#include <elfutils/libdw.h>

namespace Dyninst {
namespace DwarfDyninst {

struct dwarf_error {};

template <typename T> struct dwarf_result {
boost::optional<T> value;
bool error = false;
dwarf_result() = default;
dwarf_result(T &&t) : value{std::move(t)} {}
dwarf_result(dwarf_error) : error{true} {}
explicit operator bool() const { return !error; }
};

DYNDWARF_EXPORT dwarf_result<long> dwarf_subrange_upper_bound(Dwarf_Die *die);
DYNDWARF_EXPORT dwarf_result<long> dwarf_subrange_lower_bound(Dwarf_Die *die);
DYNDWARF_EXPORT dwarf_result<long>
dwarf_subrange_length_from_enum(Dwarf_Die *die);

} // namespace DwarfDyninst
} // namespace Dyninst