From 621bb723a4e00cb93e8a94c6126de4976dde1d9e Mon Sep 17 00:00:00 2001
From: Michael LeMay <michael.lemay@intel.com>
Date: Fri, 29 Jan 2016 12:17:13 +0800
Subject: [PATCH] BaseTools/GenFw: Enhance error message for bad symbol
 definitions

This patch expands the error message that is output when GenFw
encounters a bad symbol definition or an unsupported symbol type.  It
displays the symbol name, the symbol address, and a message that
describes both possibilities (bad symbol definition or unsupported
symbol type).  It also provides two examples of unsupported symbol
types.

Furthermore, this patch revises the conditional for detecting bad
symbol definitions to eliminate a redundant test (a Sym->st_shndx
value of SHN_ABS should certainly be greater than mEhdr->e_shnum) and
to change another test from 'Sym->st_shndx > mEhdr->e_shnum' to
'Sym->st_shndx >= mEhdr->e_shnum' for consistency with the test in
GetShdrByIndex.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael LeMay <michael.lemay@intel.com>
Reviewed-by: Yonghong Zhu <yonghong.zhu@intel.com>
---
 BaseTools/Source/C/GenFw/Elf32Convert.c | 62 +++++++++++++++++++++++--
 BaseTools/Source/C/GenFw/Elf64Convert.c | 62 +++++++++++++++++++++++--
 BaseTools/Source/C/GenFw/ElfConvert.h   |  1 +
 3 files changed, 119 insertions(+), 6 deletions(-)

diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c b/BaseTools/Source/C/GenFw/Elf32Convert.c
index dbdf05671b..41091e0888 100644
--- a/BaseTools/Source/C/GenFw/Elf32Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
@@ -266,6 +266,53 @@ IsDataShdr (
   return (BOOLEAN) (Shdr->sh_flags & (SHF_WRITE | SHF_ALLOC)) == (SHF_ALLOC | SHF_WRITE);
 }
 
+STATIC
+BOOLEAN
+IsStrtabShdr (
+  Elf_Shdr *Shdr
+  )
+{
+  Elf_Shdr *Namedr = GetShdrByIndex(mEhdr->e_shstrndx);
+
+  return (BOOLEAN) (strcmp((CHAR8*)mEhdr + Namedr->sh_offset + Shdr->sh_name, ELF_STRTAB_SECTION_NAME) == 0);
+}
+
+STATIC
+Elf_Shdr *
+FindStrtabShdr (
+  VOID
+  )
+{
+  UINT32 i;
+  for (i = 0; i < mEhdr->e_shnum; i++) {
+    Elf_Shdr *shdr = GetShdrByIndex(i);
+    if (IsStrtabShdr(shdr)) {
+      return shdr;
+    }
+  }
+  return NULL;
+}
+
+STATIC
+const UINT8 *
+GetSymName (
+  Elf_Sym *Sym
+  )
+{
+  if (Sym->st_name == 0) {
+    return NULL;
+  }
+
+  Elf_Shdr *StrtabShdr = FindStrtabShdr();
+  if (StrtabShdr == NULL) {
+    return NULL;
+  }
+
+  assert(Sym->st_name < StrtabShdr->sh_size);
+
+  return (UINT8*)mEhdr + StrtabShdr->sh_offset + Sym->st_name;
+}
+
 //
 // Elf functions interface implementation
 //
@@ -671,9 +718,18 @@ WriteSections32 (
         // header location.
         //
         if (Sym->st_shndx == SHN_UNDEF
-            || Sym->st_shndx == SHN_ABS
-            || Sym->st_shndx > mEhdr->e_shnum) {
-          Error (NULL, 0, 3000, "Invalid", "%s bad symbol definition.", mInImageName);
+            || Sym->st_shndx >= mEhdr->e_shnum) {
+          const UINT8 *SymName = GetSymName(Sym);
+          if (SymName == NULL) {
+            SymName = (const UINT8 *)"<unknown>";
+          }
+
+          Error (NULL, 0, 3000, "Invalid",
+                 "%s: Bad definition for symbol '%s'@%p or unsupported symbol type.  "
+                 "For example, absolute and undefined symbols are not supported.",
+                 mInImageName, SymName, Sym->st_value);
+
+          exit(EXIT_FAILURE);
         }
         SymShdr = GetShdrByIndex(Sym->st_shndx);
 
diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
index 974f3ca53a..5afd2ab7ca 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -258,6 +258,53 @@ IsDataShdr (
   return (BOOLEAN) (Shdr->sh_flags & (SHF_WRITE | SHF_ALLOC)) == (SHF_ALLOC | SHF_WRITE);
 }
 
+STATIC
+BOOLEAN
+IsStrtabShdr (
+  Elf_Shdr *Shdr
+  )
+{
+  Elf_Shdr *Namedr = GetShdrByIndex(mEhdr->e_shstrndx);
+
+  return (BOOLEAN) (strcmp((CHAR8*)mEhdr + Namedr->sh_offset + Shdr->sh_name, ELF_STRTAB_SECTION_NAME) == 0);
+}
+
+STATIC
+Elf_Shdr *
+FindStrtabShdr (
+  VOID
+  )
+{
+  UINT32 i;
+  for (i = 0; i < mEhdr->e_shnum; i++) {
+    Elf_Shdr *shdr = GetShdrByIndex(i);
+    if (IsStrtabShdr(shdr)) {
+      return shdr;
+    }
+  }
+  return NULL;
+}
+
+STATIC
+const UINT8 *
+GetSymName (
+  Elf_Sym *Sym
+  )
+{
+  if (Sym->st_name == 0) {
+    return NULL;
+  }
+
+  Elf_Shdr *StrtabShdr = FindStrtabShdr();
+  if (StrtabShdr == NULL) {
+    return NULL;
+  }
+
+  assert(Sym->st_name < StrtabShdr->sh_size);
+
+  return (UINT8*)mEhdr + StrtabShdr->sh_offset + Sym->st_name;
+}
+
 //
 // Elf functions interface implementation
 //
@@ -667,9 +714,18 @@ WriteSections64 (
         // header location.
         //
         if (Sym->st_shndx == SHN_UNDEF
-            || Sym->st_shndx == SHN_ABS
-            || Sym->st_shndx > mEhdr->e_shnum) {
-          Error (NULL, 0, 3000, "Invalid", "%s bad symbol definition.", mInImageName);
+            || Sym->st_shndx >= mEhdr->e_shnum) {
+          const UINT8 *SymName = GetSymName(Sym);
+          if (SymName == NULL) {
+            SymName = (const UINT8 *)"<unknown>";
+          }
+
+          Error (NULL, 0, 3000, "Invalid",
+                 "%s: Bad definition for symbol '%s'@%p or unsupported symbol type.  "
+                 "For example, absolute and undefined symbols are not supported.",
+                 mInImageName, SymName, Sym->st_value);
+
+          exit(EXIT_FAILURE);
         }
         SymShdr = GetShdrByIndex(Sym->st_shndx);
 
diff --git a/BaseTools/Source/C/GenFw/ElfConvert.h b/BaseTools/Source/C/GenFw/ElfConvert.h
index 56f165eae5..abf434dd11 100644
--- a/BaseTools/Source/C/GenFw/ElfConvert.h
+++ b/BaseTools/Source/C/GenFw/ElfConvert.h
@@ -34,6 +34,7 @@ extern UINT32 mOutImageType;
 // Common EFI specific data.
 //
 #define ELF_HII_SECTION_NAME ".hii"
+#define ELF_STRTAB_SECTION_NAME ".strtab"
 #define MAX_COFF_ALIGNMENT 0x10000
 
 //
-- 
2.39.2