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

Fix bug of unittest (related to #507) #509

Closed
wants to merge 5 commits into from

Conversation

Clcanny
Copy link
Contributor

@Clcanny Clcanny commented Dec 12, 2020

No description provided.

@Clcanny
Copy link
Contributor Author

Clcanny commented Dec 12, 2020

image

@romainthomas
Copy link
Member

It looks like it still fails (https://travis-ci.org/github/lief-project/LIEF/jobs/749209301#L1798)

@Clcanny
Copy link
Contributor Author

Clcanny commented Dec 12, 2020

It looks like it still fails (https://travis-ci.org/github/lief-project/LIEF/jobs/749209301#L1798)

Can you tell me how to run travis ci on my local machine?

@Clcanny
Copy link
Contributor Author

Clcanny commented Dec 13, 2020

I am busy right now. This issue is still on working, please wait. Thanks.

@Clcanny
Copy link
Contributor Author

Clcanny commented Dec 16, 2020

The root cause is elf generated in liefproject/manylinux1_x86_64 image doesn't has .gnu.hash section.

In Debian GNU/Linux 9.9 (stretch):

# readelf --section-headers libfoo-modified.so | grep -E "Nr|.gnu.hash" -A1
  [Nr] Name              Type             Address           Offset
       Size              EntSize          Flags  Link  Info  Align
--
  [ 2] .gnu.hash         GNU_HASH         000000000040a000  0000a000
       0000000000000054  0000000000000000   A       3     0     8

In docker container of liefproject/manylinux1_x86_64 image:

# readelf --section-headers libfoo-modified.so | grep -E "Nr|.gnu.hash" -A1
  [Nr] Name              Type             Address           Offset
       Size              EntSize          Flags  Link  Info  Align

@Clcanny Clcanny force-pushed the charles-fix-ut-bug-507 branch 3 times, most recently from 4bc7865 to 1977479 Compare December 17, 2020 07:16
@romainthomas
Copy link
Member

Ack but please make sure to pass the tests locally before using the CI

@Clcanny
Copy link
Contributor Author

Clcanny commented Dec 21, 2020

Ack but please make sure to pass the tests locally before using the CI

Hello, sorry for the interruption. And this change is okay now.

@romainthomas
Copy link
Member

Hello @Clcanny
I think that your PR is really useful nevertheless I disagree with your fix since it relies on a compiler flag and if the binary is compiled with -Wl,--hash-style=both it raises the following errors:

Symbol out-of-bound __gmon_start__
information of .dynsym section changes from 1 to 2
symndx of .gnu.hash section changes from 6 to 12
Symbol out-of-bound _ITM_registerTMCloneTable
symndx of .gnu.hash section changes from 6 to 12
Symbol out-of-bound _ITM_registerTMCloneTable
symndx of .gnu.hash section changes from 6 to 12

I had to revert the commit of your PR (see 51305e2) since it introduces breaking changes but I'll try to take a look during the holidays.

@Clcanny Clcanny marked this pull request as draft December 23, 2020 17:27
@Clcanny
Copy link
Contributor Author

Clcanny commented Dec 27, 2020

@romainthomas , sorry to disturb, wish you have a wonderful holiday.

I do the following things to improve my changes:

  1. Handle SysV hash section correctly.
  2. Let 3 python tests (SysV, GNU, both) pass.
  3. Add a detailed test report.

I still can't promise that it won't break anything, but I will try my best to fix them if any bugs found.

@Clcanny
Copy link
Contributor Author

Clcanny commented Dec 27, 2020

Test report

Environment

Docker image name: liefproject/manylinux1_x86_64

Docker image id: 6efd2f2e96ed

Code

I feel hard to debug python test case, so I write cpp test case instead.

// foo.cpp
int uninitialized_var;
int initialized_var = 1;
int add(int a, int b) {
  return a + b;
}
// main.cpp
#include <iostream>
extern int uninitialized_var;
extern int initialized_var;
int add(int a, int b);
int main() {
  std::cout << add(uninitialized_var, initialized_var) << std::endl;
}
#include <LIEF/ELF.hpp>
#include <memory>
#include <vector>
int main() {
  std::unique_ptr<LIEF::ELF::Binary> binary_(
      LIEF::ELF::Parser::parse("libfoo.so"));
  std::vector<LIEF::ELF::Symbol> dynamic_symbols{
      binary_->dynamic_symbols().begin(), binary_->dynamic_symbols().end()};
  for (const auto& symbol : dynamic_symbols) {
    binary_->add_dynamic_symbol(symbol);
  }

  auto dynsym_section = binary_->get_section(".dynsym");
  binary_->extend(dynsym_section,
                  dynamic_symbols.size() * dynsym_section.entry_size());

  if (binary_->has_section(".hash")) {
    auto hash_section = binary_->get_section(".hash");
    binary_->extend(hash_section,
                    dynamic_symbols.size() * hash_section.entry_size());
  }

  binary_->write("libfoo-modified.so");
}
# Makefile
CC=/opt/devtools-6.3/bin/gcc
CXX=/opt/devtools-6.3/bin/g++
STDLIB_PATH=/opt/devtools-6.3/lib64
hash_style=both

compile : foo.cpp main.cpp merge.cpp
        $(CC) foo.cpp -shared -fPIC -Wl,--hash-style=$(hash_style) -o libfoo.so
        $(CXX) main.cpp -Wl,--hash-style=$(hash_style) -L$(PWD) -Wl,-rpath=$(PWD) -lfoo -o main
        $(CXX) merge.cpp -std=c++11 -I/usr/include/LIEF -L/usr/lib -lLIEF -o merge

run : compile libfoo.so main merge
        LD_LIBRARY_PATH=$(STDLIB_PATH) ./merge

test : run libfoo.so libfoo-modified.so main
        ./test.sh
#!/bin/bash

export origin_hash_section_start_addr="0x$(readelf --section-headers libfoo.so | grep ' .hash' | awk '{print $6}')"
export origin_hash_section_size="0x$(readelf --section-headers libfoo.so | grep ' .hash' -A1 | tail -n 1 | awk '{print $1}')"
export origin_nbucket="0x$(od --skip-bytes=$origin_hash_section_start_addr --read-bytes=8 --format=xI libfoo.so | head -n 1 | awk '{print $2}')"
export origin_nchain="0x$(od --skip-bytes=$origin_hash_section_start_addr --read-bytes=8 --format=xI libfoo.so | head -n 1 | awk '{print $3}')"

rm libfoo.so
chmod u+x libfoo-modified.so
mv libfoo-modified.so libfoo.so
if ./main | grep -q "1"
then
    echo "Run main successfully."
fi

export new_hash_section_start_addr="0x$(readelf --section-headers libfoo.so | grep ' .hash' | awk '{print $6}')"
export new_hash_section_size="0x$(readelf --section-headers libfoo.so | grep ' .hash' -A1 | tail -n 1 | awk '{print $1}')"
export new_nbucket="0x$(od --skip-bytes=$new_hash_section_start_addr --read-bytes=8 --format=xI libfoo.so | head -n 1 | awk '{print $2}')"
export new_nchain="0x$(od --skip-bytes=$new_hash_section_start_addr --read-bytes=8 --format=xI libfoo.so | head -n 1 | awk '{print $3}')"

if [[ $new_nbucket -eq $origin_nbucket ]]
then
    echo "new_bucket is equal to origin_nbucket."
fi

if [[ $new_nchain -eq $((origin_nchain * 2)) ]]
then
    echo "new_nchain is twice origin_nchain."
fi

if [[ $origin_hash_section_size -eq $((4 + 4 + origin_nbucket * 4 + origin_nchain * 4)) ]]
then
    echo "origin_hash_section_size is expected."
fi

if [[ $new_hash_section_size -eq $((4 + 4 + new_nbucket * 4 + new_nchain * 4)) ]]
then
    echo "new_hash_section_size is expected."
fi

Symbol orders are expected.

SysV

# make test hash_style=sysv
/opt/devtools-6.3/bin/gcc foo.cpp -shared -fPIC -Wl,--hash-style=sysv -o libfoo.so
/opt/devtools-6.3/bin/g++ main.cpp -Wl,--hash-style=sysv -L/root/test_compatibility_of_hash_table -Wl,-rpath=/root/test_compatibility_of_hash_table -lfoo -o main
# readelf --dyn-syms libfoo.so
Symbol table '.dynsym' contains 28 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
     1: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND __gmon_start__
     2: 00000000000027dc     0 FUNC    GLOBAL DEFAULT   11 _fini
     3: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _Jv_RegisterClasses
     4: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _ITM_deregisterTMCloneTab
     5: 0000000000002678     0 FUNC    GLOBAL DEFAULT    7 _init
     6: 00000000002031a8     4 OBJECT  GLOBAL DEFAULT   20 initialized_var
     7: 00000000000027c8    20 FUNC    GLOBAL DEFAULT   10 _Z3addii
     8: 00000000002031b0     4 OBJECT  GLOBAL DEFAULT   21 uninitialized_var
     9: 00000000002031ac     0 NOTYPE  GLOBAL DEFAULT   21 __bss_start
    10: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _ITM_registerTMCloneTable
    11: 0000000000000000     0 FUNC    WEAK   DEFAULT  UND __cxa_finalize@GLIBC_2.2.5 (2)
    12: 00000000002031ac     0 NOTYPE  GLOBAL DEFAULT   20 _edata
    13: 00000000002031b8     0 NOTYPE  GLOBAL DEFAULT   21 _end
    14: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
    15: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND __gmon_start__
    16: 00000000000027dc     0 FUNC    GLOBAL DEFAULT   11 _fini
    17: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _Jv_RegisterClasses
    18: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _ITM_deregisterTMCloneTab
    19: 0000000000002678     0 FUNC    GLOBAL DEFAULT    7 _init
    20: 00000000002031a8     4 OBJECT  GLOBAL DEFAULT   20 initialized_var
    21: 00000000000027c8    20 FUNC    GLOBAL DEFAULT   10 _Z3addii
    22: 00000000002031b0     4 OBJECT  GLOBAL DEFAULT   21 uninitialized_var
    23: 00000000002031ac     0 NOTYPE  GLOBAL DEFAULT   21 __bss_start
    24: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _ITM_registerTMCloneTable
    25: 0000000000000000     0 FUNC    WEAK   DEFAULT  UND __cxa_finalize
    26: 00000000002031ac     0 NOTYPE  GLOBAL DEFAULT   20 _edata
    27: 00000000002031b8     0 NOTYPE  GLOBAL DEFAULT   21 _end

They just keep the same order, I don't sort them. It is expetced.

GNU

# make test hash_style=gnu
/opt/devtools-6.3/bin/gcc foo.cpp -shared -fPIC -Wl,--hash-style=gnu -o libfoo.so
/opt/devtools-6.3/bin/g++ main.cpp -Wl,--hash-style=gnu -L/root/test_compatibility_of_hash_table -Wl,-rpath=/root/test_compatibility_of_hash_table -lfoo -o main
# readelf --dyn-syms libfoo.so
Symbol table '.dynsym' contains 28 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
     1: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
     2: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND __gmon_start__
     3: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _Jv_RegisterClasses
     4: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _ITM_deregisterTMCloneTab
     5: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _ITM_registerTMCloneTable
     6: 0000000000000000     0 FUNC    WEAK   DEFAULT  UND __cxa_finalize@GLIBC_2.2.5 (2)
     7: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND __gmon_start__
     8: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _Jv_RegisterClasses
     9: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _ITM_deregisterTMCloneTab
    10: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _ITM_registerTMCloneTable
    11: 0000000000000000     0 FUNC    WEAK   DEFAULT  UND __cxa_finalize
    12: 0000000000204178     4 OBJECT  GLOBAL DEFAULT   21 uninitialized_var
    13: 0000000000204174     0 NOTYPE  GLOBAL DEFAULT   20 _edata
    14: 0000000000204180     0 NOTYPE  GLOBAL DEFAULT   21 _end
    15: 0000000000204178     4 OBJECT  GLOBAL DEFAULT   21 uninitialized_var
    16: 0000000000204174     0 NOTYPE  GLOBAL DEFAULT   20 _edata
    17: 0000000000204180     0 NOTYPE  GLOBAL DEFAULT   21 _end
    18: 0000000000003638     0 FUNC    GLOBAL DEFAULT    7 _init
    19: 0000000000204174     0 NOTYPE  GLOBAL DEFAULT   21 __bss_start
    20: 0000000000003638     0 FUNC    GLOBAL DEFAULT    7 _init
    21: 0000000000204174     0 NOTYPE  GLOBAL DEFAULT   21 __bss_start
    22: 00000000000037a4     0 FUNC    GLOBAL DEFAULT   11 _fini
    23: 0000000000204170     4 OBJECT  GLOBAL DEFAULT   20 initialized_var
    24: 0000000000003790    20 FUNC    GLOBAL DEFAULT   10 _Z3addii
    25: 00000000000037a4     0 FUNC    GLOBAL DEFAULT   11 _fini
    26: 0000000000204170     4 OBJECT  GLOBAL DEFAULT   20 initialized_var
    27: 0000000000003790    20 FUNC    GLOBAL DEFAULT   10 _Z3addii

They are sorted, and it is expected.

Both

# make test hash_style=both
/opt/devtools-6.3/bin/gcc foo.cpp -shared -fPIC -Wl,--hash-style=both -o libfoo.so
/opt/devtools-6.3/bin/g++ main.cpp -Wl,--hash-style=both -L/root/test_compatibility_of_hash_table -Wl,-rpath=/root/test_compatibility_of_hash_table -lfoo -o main
# readelf --dyn-syms libfoo.so
Symbol table '.dynsym' contains 28 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
     1: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
     2: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND __gmon_start__
     3: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _Jv_RegisterClasses
     4: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _ITM_deregisterTMCloneTab
     5: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _ITM_registerTMCloneTable
     6: 0000000000000000     0 FUNC    WEAK   DEFAULT  UND __cxa_finalize@GLIBC_2.2.5 (2)
     7: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND __gmon_start__
     8: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _Jv_RegisterClasses
     9: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _ITM_deregisterTMCloneTab
    10: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _ITM_registerTMCloneTable
    11: 0000000000000000     0 FUNC    WEAK   DEFAULT  UND __cxa_finalize
    12: 00000000002041b0     4 OBJECT  GLOBAL DEFAULT   22 uninitialized_var
    13: 00000000002041ac     0 NOTYPE  GLOBAL DEFAULT   21 _edata
    14: 00000000002041b8     0 NOTYPE  GLOBAL DEFAULT   22 _end
    15: 00000000002041b0     4 OBJECT  GLOBAL DEFAULT   22 uninitialized_var
    16: 00000000002041ac     0 NOTYPE  GLOBAL DEFAULT   21 _edata
    17: 00000000002041b8     0 NOTYPE  GLOBAL DEFAULT   22 _end
    18: 00000000000036c0     0 FUNC    GLOBAL DEFAULT    8 _init
    19: 00000000002041ac     0 NOTYPE  GLOBAL DEFAULT   22 __bss_start
    20: 00000000000036c0     0 FUNC    GLOBAL DEFAULT    8 _init
    21: 00000000002041ac     0 NOTYPE  GLOBAL DEFAULT   22 __bss_start
    22: 000000000000382c     0 FUNC    GLOBAL DEFAULT   12 _fini
    23: 00000000002041a8     4 OBJECT  GLOBAL DEFAULT   21 initialized_var
    24: 0000000000003818    20 FUNC    GLOBAL DEFAULT   11 _Z3addii
    25: 000000000000382c     0 FUNC    GLOBAL DEFAULT   12 _fini
    26: 00000000002041a8     4 OBJECT  GLOBAL DEFAULT   21 initialized_var
    27: 0000000000003818    20 FUNC    GLOBAL DEFAULT   11 _Z3addii

They are sorted, and it is expected.

Sizes of .hash section are expected.

When you run make test hash_style=sysv and make test hash_style=both, they will say:

Run main successfully.
new_bucket is equal to origin_nbucket.
new_nchain is twice origin_nchain.
origin_hash_section_size is expected.
new_hash_section_size is expected.

@Clcanny Clcanny marked this pull request as ready for review December 27, 2020 19:26
@Clcanny
Copy link
Contributor Author

Clcanny commented Jan 27, 2021

@romainthomas Hello, I notice that the new version is released. Is it a good time to merge this pull request? Thank you.

@romainthomas
Copy link
Member

Yes I didn't forget, I'll do that in the next couple of weeks

@romainthomas
Copy link
Member

done with 3d0da01

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants