[Lldb-commits] [lldb] [lldb] Fix a off-by-one error in ParseSupportFilesFromPrologue (PR #71984)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/71984

This fixes a subtle and previously harmless off-by-one bug in ParseSupportFilesFromPrologue. The function accounts for the start index being one-based for DWARF v4 and earlier and zero-based for DWARF v5 and later. However, the same care wasn't taken for the end index.

This bug existed unnoticed because GetFileByIndex gracefully handles an invalid index. However, the bug manifested itself after #71458, which added a call to getFileNameEntry which requires the index to be valid.

No test as the bug cannot be observed without the changes from #71458. One that PR is merged, this will be covered by all the DWARF v5 tests.

>From c6e6d30da2e67b2de53211fd70a7077a7dab62ef Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Fri, 10 Nov 2023 12:44:18 -0800
Subject: [PATCH] [lldb] Fix a off-by-one error in
 ParseSupportFilesFromPrologue

This fixes a subtle and previously harmless off-by-one bug in
ParseSupportFilesFromPrologue. The function accounts for the start index
being one-based for DWARF v4 and earlier and zero-based for DWARF v5 and
later. However, the same care wasn't taken for the end index.

This bug existed unnoticed because GetFileByIndex gracefully handles an
invalid index. However, the bug manifested itself after #71458, which
added a call to getFileNameEntry which requires the index to be valid.

No test as the bug cannot be observed without the changes from #71458.
One that PR is merged, this will be covered by all the DWARF v5 tests.
---
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp      | 25 ++++++++++++-------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index aabd83a194932ff..67ea17b2e1fb1bd 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -210,17 +210,24 @@ ParseSupportFilesFromPrologue(const lldb::ModuleSP &module,
                               FileSpec::Style style,
                               llvm::StringRef compile_dir = {}) {
   FileSpecList support_files;
-  size_t first_file = 0;
-  if (prologue.getVersion() <= 4) {
-    // File index 0 is not valid before DWARF v5. Add a dummy entry to ensure
-    // support file list indices match those we get from the debug info and line
-    // tables.
+
+  // Handle the case where there are no files first to avoid having to special
+  // case this later.
+  if (prologue.FileNames.empty())
+    return support_files;
+
+  // Before DWARF v5, the line table indexes were one based.
+  const bool is_one_based = prologue.getVersion() < 5;
+  const size_t file_names = prologue.FileNames.size();
+  const size_t first_file_idx = is_one_based ? 1 : 0;
+  const size_t last_file_idx = is_one_based ? file_names : file_names - 1;
+
+  // Add a dummy entry to ensure the support file list indices match those we
+  // get from the debug info and line tables.
+  if (is_one_based)
     support_files.Append(FileSpec());
-    first_file = 1;
-  }
 
-  const size_t number_of_files = prologue.FileNames.size();
-  for (size_t idx = first_file; idx <= number_of_files; ++idx) {
+  for (size_t idx = first_file_idx; idx <= last_file_idx; ++idx) {
     std::string remapped_file;
     if (auto file_path = GetFileByIndex(prologue, idx, compile_dir, style)) {
       if (auto remapped = module->RemapSourceFile(llvm::StringRef(*file_path)))