LibDebug+LibCoredump: Replace remaining reinterpret_casts and C casts

You misused your toys and I'm now taking them away, reflect on what you
did wrong for a bit.

Userland/Libraries/LibCoredump/Backtrace.cpp

@@ -46,32 +46,32 @@ Backtrace::Backtrace(const Reader& coredump, const ELF::Core::ThreadInfo& thread
     : m_thread_info(move(thread_info))
 {
 #if ARCH(I386)
-    auto* start_bp = (FlatPtr*)m_thread_info.regs.ebp;
-    auto* start_ip = (FlatPtr*)m_thread_info.regs.eip;
+    auto start_bp = m_thread_info.regs.ebp;
+    auto start_ip = m_thread_info.regs.eip;
 #else
-    auto* start_bp = (FlatPtr*)m_thread_info.regs.rbp;
-    auto* start_ip = (FlatPtr*)m_thread_info.regs.rip;
+    auto start_bp = m_thread_info.regs.rbp;
+    auto start_ip = m_thread_info.regs.rip;
 #endif
 
     // In order to provide progress updates, we first have to walk the
     // call stack to determine how many frames it has.
     size_t frame_count = 0;
     {
-        auto* bp = start_bp;
-        auto* ip = start_ip;
+        auto bp = start_bp;
+        auto ip = start_ip;
         while (bp && ip) {
             ++frame_count;
-            auto next_ip = coredump.peek_memory((FlatPtr)(bp + 1));
-            auto next_bp = coredump.peek_memory((FlatPtr)(bp));
+            auto next_ip = coredump.peek_memory(bp + sizeof(FlatPtr));
+            auto next_bp = coredump.peek_memory(bp);
             if (!next_ip.has_value() || !next_bp.has_value())
                 break;
-            ip = (FlatPtr*)next_ip.value();
-            bp = (FlatPtr*)next_bp.value();
+            ip = next_ip.value();
+            bp = next_bp.value();
         }
     }
 
-    auto* bp = start_bp;
-    auto* ip = start_ip;
+    auto bp = start_bp;
+    auto ip = start_ip;
     size_t frame_index = 0;
     while (bp && ip) {
         // We use eip - 1 because the return address from a function frame
@@ -79,17 +79,17 @@ Backtrace::Backtrace(const Reader& coredump, const ELF::Core::ThreadInfo& thread
         // However, because the first frame represents the faulting
         // instruction rather than the return address we don't subtract
         // 1 there.
-        VERIFY((FlatPtr)ip > 0);
-        add_entry(coredump, (FlatPtr)ip - ((frame_index == 0) ? 0 : 1));
+        VERIFY(ip > 0);
+        add_entry(coredump, ip - ((frame_index == 0) ? 0 : 1));
         if (on_progress)
             on_progress(frame_index, frame_count);
         ++frame_index;
-        auto next_ip = coredump.peek_memory((FlatPtr)(bp + 1));
-        auto next_bp = coredump.peek_memory((FlatPtr)(bp));
+        auto next_ip = coredump.peek_memory(bp + sizeof(FlatPtr));
+        auto next_bp = coredump.peek_memory(bp);
         if (!next_ip.has_value() || !next_bp.has_value())
             break;
-        ip = (FlatPtr*)next_ip.value();
-        bp = (FlatPtr*)next_bp.value();
+        ip = next_ip.value();
+        bp = next_bp.value();
     }
 }
 
@@ -99,7 +99,7 @@ Backtrace::~Backtrace()
 
 void Backtrace::add_entry(const Reader& coredump, FlatPtr ip)
 {
-    auto* ip_region = coredump.region_containing((FlatPtr)ip);
+    auto* ip_region = coredump.region_containing(ip);
     if (!ip_region) {
         m_entries.append({ ip, {}, {}, {} });
         return;

Userland/Libraries/LibCoredump/Inspector.cpp

@@ -39,7 +39,7 @@ void Inspector::parse_loaded_libraries(Function<void(float)> on_progress)
     m_reader->for_each_library([this, number_of_libraries, &library_index, &on_progress](Coredump::Reader::LibraryInfo library) {
         ++library_index;
         if (on_progress)
-            on_progress(library_index / (float)number_of_libraries);
+            on_progress(library_index / static_cast<float>(number_of_libraries));
 
         auto file_or_error = Core::MappedFile::map(library.path);
         if (file_or_error.is_error())

Userland/Libraries/LibCoredump/Reader.cpp

@@ -79,7 +79,7 @@ Reader::~Reader()
 }
 
 Reader::NotesEntryIterator::NotesEntryIterator(const u8* notes_data)
-    : m_current((const ELF::Core::NotesEntry*)notes_data)
+    : m_current(bit_cast<const ELF::Core::NotesEntry*>(notes_data))
     , start(notes_data)
 {
 }
@@ -104,23 +104,23 @@ void Reader::NotesEntryIterator::next()
     VERIFY(!at_end());
     switch (type()) {
     case ELF::Core::NotesEntryHeader::Type::ProcessInfo: {
-        const auto* current = reinterpret_cast<const ELF::Core::ProcessInfo*>(m_current);
-        m_current = reinterpret_cast<const ELF::Core::NotesEntry*>(current->json_data + strlen(current->json_data) + 1);
+        const auto* current = bit_cast<const ELF::Core::ProcessInfo*>(m_current);
+        m_current = bit_cast<const ELF::Core::NotesEntry*>(current->json_data + strlen(current->json_data) + 1);
         break;
     }
     case ELF::Core::NotesEntryHeader::Type::ThreadInfo: {
-        const auto* current = reinterpret_cast<const ELF::Core::ThreadInfo*>(m_current);
-        m_current = reinterpret_cast<const ELF::Core::NotesEntry*>(current + 1);
+        const auto* current = bit_cast<const ELF::Core::ThreadInfo*>(m_current);
+        m_current = bit_cast<const ELF::Core::NotesEntry*>(current + 1);
         break;
     }
     case ELF::Core::NotesEntryHeader::Type::MemoryRegionInfo: {
-        const auto* current = reinterpret_cast<const ELF::Core::MemoryRegionInfo*>(m_current);
-        m_current = reinterpret_cast<const ELF::Core::NotesEntry*>(current->region_name + strlen(current->region_name) + 1);
+        const auto* current = bit_cast<const ELF::Core::MemoryRegionInfo*>(m_current);
+        m_current = bit_cast<const ELF::Core::NotesEntry*>(current->region_name + strlen(current->region_name) + 1);
         break;
     }
     case ELF::Core::NotesEntryHeader::Type::Metadata: {
-        const auto* current = reinterpret_cast<const ELF::Core::Metadata*>(m_current);
-        m_current = reinterpret_cast<const ELF::Core::NotesEntry*>(current->json_data + strlen(current->json_data) + 1);
+        const auto* current = bit_cast<const ELF::Core::Metadata*>(m_current);
+        m_current = bit_cast<const ELF::Core::NotesEntry*>(current->json_data + strlen(current->json_data) + 1);
         break;
     }
     default:
@@ -149,10 +149,11 @@ Optional<FlatPtr> Reader::peek_memory(FlatPtr address) const
 const JsonObject Reader::process_info() const
 {
     const ELF::Core::ProcessInfo* process_info_notes_entry = nullptr;
-    for (NotesEntryIterator it((const u8*)m_coredump_image.program_header(m_notes_segment_index).raw_data()); !it.at_end(); it.next()) {
+    NotesEntryIterator it(bit_cast<const u8*>(m_coredump_image.program_header(m_notes_segment_index).raw_data()));
+    for (; !it.at_end(); it.next()) {
         if (it.type() != ELF::Core::NotesEntryHeader::Type::ProcessInfo)
             continue;
-        process_info_notes_entry = reinterpret_cast<const ELF::Core::ProcessInfo*>(it.current());
+        process_info_notes_entry = bit_cast<const ELF::Core::ProcessInfo*>(it.current());
         break;
     }
     if (!process_info_notes_entry)
@@ -203,10 +204,10 @@ u8 Reader::process_termination_signal() const
 {
     auto process_info = this->process_info();
     auto termination_signal = process_info.get("termination_signal");
-    auto signal_number = termination_signal.to_number<int>();
+    auto signal_number = termination_signal.to_number<u8>();
     if (signal_number <= SIGINVAL || signal_number >= NSIG)
         return SIGINVAL;
-    return (u8)signal_number;
+    return signal_number;
 }
 
 String Reader::process_executable_path() const
@@ -247,10 +248,11 @@ Vector<String> Reader::process_environment() const
 HashMap<String, String> Reader::metadata() const
 {
     const ELF::Core::Metadata* metadata_notes_entry = nullptr;
-    for (NotesEntryIterator it((const u8*)m_coredump_image.program_header(m_notes_segment_index).raw_data()); !it.at_end(); it.next()) {
+    NotesEntryIterator it(bit_cast<const u8*>(m_coredump_image.program_header(m_notes_segment_index).raw_data()));
+    for (; !it.at_end(); it.next()) {
         if (it.type() != ELF::Core::NotesEntryHeader::Type::Metadata)
             continue;
-        metadata_notes_entry = reinterpret_cast<const ELF::Core::Metadata*>(it.current());
+        metadata_notes_entry = bit_cast<const ELF::Core::Metadata*>(it.current());
         break;
     }
     if (!metadata_notes_entry)
@@ -267,11 +269,6 @@ HashMap<String, String> Reader::metadata() const
     return metadata;
 }
 
-struct LibraryData {
-    String name;
-    ELF::Image lib_elf;
-};
-
 const Reader::LibraryData* Reader::library_containing(FlatPtr address) const
 {
     static HashMap<String, OwnPtr<LibraryData>> cached_libs;
@@ -293,7 +290,7 @@ const Reader::LibraryData* Reader::library_containing(FlatPtr address) const
         if (file_or_error.is_error())
             return {};
         auto image = ELF::Image(file_or_error.value()->bytes());
-        cached_libs.set(path, make<LibraryData>(name, (FlatPtr)region->region_start, file_or_error.release_value(), move(image)));
+        cached_libs.set(path, make<LibraryData>(name, static_cast<FlatPtr>(region->region_start), file_or_error.release_value(), move(image)));
     }
 
     auto lib_data = cached_libs.get(path).value();
@@ -317,7 +314,7 @@ void Reader::for_each_library(Function<void(LibraryInfo)> func) const
             path = name;
         }
 
-        func(LibraryInfo { name, path, (FlatPtr)region.region_start });
+        func(LibraryInfo { name, path, static_cast<FlatPtr>(region.region_start) });
         return IterationDecision::Continue;
     });
 }

Userland/Libraries/LibCoredump/Reader.h

@@ -100,10 +100,11 @@ private:
 template<typename Func>
 void Reader::for_each_memory_region_info(Func func) const
 {
-    for (NotesEntryIterator it((const u8*)m_coredump_image.program_header(m_notes_segment_index).raw_data()); !it.at_end(); it.next()) {
+    NotesEntryIterator it(bit_cast<const u8*>(m_coredump_image.program_header(m_notes_segment_index).raw_data()));
+    for (; !it.at_end(); it.next()) {
         if (it.type() != ELF::Core::NotesEntryHeader::Type::MemoryRegionInfo)
             continue;
-        auto& memory_region_info = reinterpret_cast<const ELF::Core::MemoryRegionInfo&>(*it.current());
+        auto& memory_region_info = *bit_cast<const ELF::Core::MemoryRegionInfo*>(it.current());
         IterationDecision decision = func(memory_region_info);
         if (decision == IterationDecision::Break)
             return;
@@ -113,10 +114,11 @@ void Reader::for_each_memory_region_info(Func func) const
 template<typename Func>
 void Reader::for_each_thread_info(Func func) const
 {
-    for (NotesEntryIterator it((const u8*)m_coredump_image.program_header(m_notes_segment_index).raw_data()); !it.at_end(); it.next()) {
+    NotesEntryIterator it(bit_cast<const u8*>(m_coredump_image.program_header(m_notes_segment_index).raw_data()));
+    for (; !it.at_end(); it.next()) {
         if (it.type() != ELF::Core::NotesEntryHeader::Type::ThreadInfo)
             continue;
-        auto& thread_info = reinterpret_cast<const ELF::Core::ThreadInfo&>(*it.current());
+        auto& thread_info = *bit_cast<const ELF::Core::ThreadInfo*>(it.current());
         IterationDecision decision = func(thread_info);
         if (decision == IterationDecision::Break)
             return;

Userland/Libraries/LibDebug/DebugInfo.cpp

@@ -231,7 +231,7 @@ static void parse_variable_location(Dwarf::DIE const& variable_die, DebugInfo::V
         break;
     }
     default:
-        dbgln("Warning: unhandled Dwarf location type: {}", (int)location_info.value().type());
+        dbgln("Warning: unhandled Dwarf location type: {}", to_underlying(location_info.value().type()));
     }
 }
 
@@ -429,14 +429,16 @@ Optional<Dwarf::LineProgram::DirectoryAndFile> DebugInfo::get_source_path_of_inl
 {
     auto caller_file = die.get_attribute(Dwarf::Attribute::CallFile);
     if (caller_file.has_value()) {
-        u32 file_index = 0;
+        size_t file_index = 0;
 
         if (caller_file->type() == Dwarf::AttributeValue::Type::UnsignedNumber) {
             file_index = caller_file->as_unsigned();
         } else if (caller_file->type() == Dwarf::AttributeValue::Type::SignedNumber) {
             // For some reason, the file_index is sometimes stored as a signed number.
-            VERIFY(caller_file->as_signed() >= 0);
-            file_index = (u32)caller_file->as_signed();
+            auto signed_file_index = caller_file->as_signed();
+            VERIFY(signed_file_index >= 0);
+            VERIFY(static_cast<u64>(signed_file_index) <= NumericLimits<size_t>::max());
+            file_index = static_cast<size_t>(caller_file->as_signed());
         } else {
             return {};
         }

Userland/Libraries/LibDebug/DebugSession.cpp

@@ -80,11 +80,11 @@ OwnPtr<DebugSession> DebugSession::exec_and_attach(String const& command,
 
         auto parts = command.split(' ');
         VERIFY(!parts.is_empty());
-        const char** args = (const char**)calloc(parts.size() + 1, sizeof(const char*));
+        const char** args = bit_cast<const char**>(calloc(parts.size() + 1, sizeof(const char*)));
         for (size_t i = 0; i < parts.size(); i++) {
             args[i] = parts[i].characters();
         }
-        const char** envp = (const char**)calloc(2, sizeof(const char*));
+        const char** envp = bit_cast<const char**>(calloc(2, sizeof(const char*)));
         // This causes loader to stop on a breakpoint before jumping to the entry point of the program.
         envp[0] = "_LOADER_BREAKPOINT=1";
         int rc = execvpe(args[0], const_cast<char**>(args), const_cast<char**>(envp));

Userland/Libraries/LibDebug/Dwarf/AddressRanges.cpp

@@ -80,7 +80,7 @@ void AddressRangesV5::for_each_range(Function<void(Range)> callback)
         case RangeListEntryType::EndOfList:
             return;
         default:
-            dbgln("unsupported range list entry type: 0x{:x}", (int)entry_type);
+            dbgln("unsupported range list entry type: 0x{:x}", entry_type);
             return;
         }
     }

Userland/Libraries/LibDebug/Dwarf/DwarfInfo.cpp

@@ -84,7 +84,7 @@ AttributeValue DwarfInfo::get_attribute_value(AttributeDataForm form, ssize_t im
     value.m_compilation_unit = unit;
 
     auto assign_raw_bytes_value = [&](size_t length) {
-        value.m_data.as_raw_bytes = { reinterpret_cast<const u8*>(debug_info_data().data() + debug_info_stream.offset()), length };
+        value.m_data.as_raw_bytes = { debug_info_data().offset_pointer(debug_info_stream.offset()), length };
 
         debug_info_stream.discard_or_error(length);
         VERIFY(!debug_info_stream.has_any_error());
@@ -98,7 +98,7 @@ AttributeValue DwarfInfo::get_attribute_value(AttributeDataForm form, ssize_t im
         value.m_type = AttributeValue::Type::String;
 
         auto strings_data = debug_strings_data();
-        value.m_data.as_string = reinterpret_cast<const char*>(strings_data.data() + offset);
+        value.m_data.as_string = bit_cast<const char*>(strings_data.offset_pointer(offset));
         break;
     }
     case AttributeDataForm::Data1: {
@@ -199,7 +199,7 @@ AttributeValue DwarfInfo::get_attribute_value(AttributeDataForm form, ssize_t im
         debug_info_stream >> str;
         VERIFY(!debug_info_stream.has_any_error());
         value.m_type = AttributeValue::Type::String;
-        value.m_data.as_string = reinterpret_cast<const char*>(str_offset + debug_info_data().data());
+        value.m_data.as_string = bit_cast<const char*>(debug_info_data().offset_pointer(str_offset));
         break;
     }
     case AttributeDataForm::Block1: {
@@ -241,7 +241,7 @@ AttributeValue DwarfInfo::get_attribute_value(AttributeDataForm form, ssize_t im
         value.m_type = AttributeValue::Type::String;
 
         auto strings_data = debug_line_strings_data();
-        value.m_data.as_string = reinterpret_cast<const char*>(strings_data.data() + offset);
+        value.m_data.as_string = bit_cast<const char*>(strings_data.offset_pointer(offset));
         break;
     }
     case AttributeDataForm::ImplicitConst: {
@@ -323,7 +323,7 @@ AttributeValue DwarfInfo::get_attribute_value(AttributeDataForm form, ssize_t im
         break;
     }
     default:
-        dbgln("Unimplemented AttributeDataForm: {}", (u32)form);
+        dbgln("Unimplemented AttributeDataForm: {}", to_underlying(form));
         VERIFY_NOT_REACHED();
     }
     return value;

Userland/Libraries/LibDebug/Dwarf/DwarfTypes.h

@@ -47,7 +47,10 @@ struct [[gnu::packed]] CompilationUnitHeader {
 
     u32 length() const { return common.length; }
     u16 version() const { return common.version; }
-    CompilationUnitType unit_type() const { return (common.version <= 4) ? CompilationUnitType::Full : (CompilationUnitType)v5.unit_type; }
+    CompilationUnitType unit_type() const
+    {
+        return (common.version <= 4) ? CompilationUnitType::Full : static_cast<CompilationUnitType>(v5.unit_type);
+    }
     u32 abbrev_offset() const { return (common.version <= 4) ? v4.abbrev_offset : v5.abbrev_offset; }
     u8 address_size() const { return (common.version <= 4) ? v4.address_size : v5.address_size; }
 };

Userland/Libraries/LibDebug/Dwarf/LineProgram.cpp

@@ -42,13 +42,13 @@ void LineProgram::parse_path_entries(Function<void(PathEntry& entry)> callback,
         Vector<PathEntryFormat> format_descriptions;
 
         for (u8 i = 0; i < path_entry_format_count; i++) {
-            size_t content_type = 0;
+            UnderlyingType<ContentType> content_type;
             m_stream.read_LEB128_unsigned(content_type);
 
-            size_t data_form = 0;
+            UnderlyingType<AttributeDataForm> data_form;
             m_stream.read_LEB128_unsigned(data_form);
 
-            format_descriptions.empend((ContentType)content_type, (AttributeDataForm)data_form);
+            format_descriptions.empend(static_cast<ContentType>(content_type), static_cast<AttributeDataForm>(data_form));
         }
 
         size_t paths_count = 0;
@@ -66,7 +66,7 @@ void LineProgram::parse_path_entries(Function<void(PathEntry& entry)> callback,
                     entry.directory_index = value.as_unsigned();
                     break;
                 default:
-                    dbgln_if(DWARF_DEBUG, "Unhandled path list attribute: {}", (int)format_description.type);
+                    dbgln_if(DWARF_DEBUG, "Unhandled path list attribute: {}", to_underlying(format_description.type));
                 }
             }
             callback(entry);
@@ -280,7 +280,7 @@ void LineProgram::run_program()
 {
     reset_registers();
 
-    while ((size_t)m_stream.offset() < m_unit_offset + sizeof(u32) + m_unit_header.length()) {
+    while (m_stream.offset() < m_unit_offset + sizeof(u32) + m_unit_header.length()) {
         u8 opcode = 0;
         m_stream >> opcode;