Issue found by valgrind

[Quuxplusone]

Bugzilla Link	PR17595
Status	RESOLVED FIXED
Importance	P normal
Reported by	octoploid (octoploid@yandex.com)
Reported on	2013-10-16 03:07:21 -0700
Last modified on	2013-10-16 07:48:31 -0700
Version	trunk
Hardware	PC Linux
CC	isanbard@gmail.com, llvm-bugs@lists.llvm.org, rafael@espindo.la
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

~ % valgrind --track-origins=yes --trace-children=yes clang++ -flto -O2
bench.cpp
...
==23495==
==23495== Invalid read of size 1
==23495==    at 0x402B120: strlen (mc_replace_strmem.c:399)
==23495==    by 0x674F64: std::basic_string<char, std::char_traits<char>,
std::allocator<char> >::basic_string(char const*, std::allocator<char> const&)
(in /usr/x86_64-pc-linux-gnu/binutils-bin/git/ld)
==23495==    by 0x5B8617: gold::Plugin_manager::add_input_file(char const*,
bool) (options.h:1717)
==23495==    by 0x41EE5EA: all_symbols_read_hook() (gold-plugin.cpp:444)
==23495==    by 0x5B7946:
gold::Plugin_manager::all_symbols_read(gold::Workqueue*, gold::Task*,
gold::Input_objects*, gold::Symbol_table*, gold::Dirsearch*, gold::Mapfile*, g
old::Task_token**) (plugin.cc:359)
==23495==    by 0x5B7A73: gold::Plugin_hook::run(gold::Workqueue*)
(plugin.cc:1420)
==23495==    by 0x609C3C: gold::Workqueue::find_and_run_task(int)
(workqueue.cc:319)
==23495==    by 0x609FC9: gold::Workqueue::process(int) (workqueue.cc:495)
==23495==    by 0x4069EF: main (main.cc:252)
==23495==  Address 0x544f1e8 is 24 bytes inside a block of size 47 free'd
==23495==    at 0x4028FF8: operator delete(void*) (vg_replace_malloc.c:480)
==23495==    by 0x5D3C92C: LTOCodeGenerator::~LTOCodeGenerator()
(basic_string.h:249)
==23495==    by 0x5C6121D: lto_codegen_dispose (lto.cpp:213)
==23495==    by 0x41EE56B: all_symbols_read_hook() (gold-plugin.cpp:435)
==23495==    by 0x5B7946:
gold::Plugin_manager::all_symbols_read(gold::Workqueue*, gold::Task*,
gold::Input_objects*, gold::Symbol_table*, gold::Dirsearch*, gold::Mapfile*, g
old::Task_token**) (plugin.cc:359)
==23495==    by 0x5B7A73: gold::Plugin_hook::run(gold::Workqueue*)
(plugin.cc:1420)
==23495==    by 0x609C3C: gold::Workqueue::find_and_run_task(int)
(workqueue.cc:319)
==23495==    by 0x609FC9: gold::Workqueue::process(int) (workqueue.cc:495)
==23495==    by 0x4069EF: main (main.cc:252)
...
==23495== Invalid read of size 8
==23495==    at 0x402C070: memcpy@@GLIBC_2.14 (mc_replace_strmem.c:877)
==23495==    by 0x674C6D: char* std::string::_S_construct<char const*>(char
const*, char const*, std::allocator<char> const&, std::forward_iterator_tag)
(in /usr/x86_64-pc-li
nux-gnu/binutils-bin/git/ld)
==23495==    by 0x674F79: std::basic_string<char, std::char_traits<char>,
std::allocator<char> >::basic_string(char const*, std::allocator<char> const&)
(in /usr/x86_64-pc-li
nux-gnu/binutils-bin/git/ld)
==23495==    by 0x5B8617: gold::Plugin_manager::add_input_file(char const*,
bool) (options.h:1717)
==23495==    by 0x41EE5EA: all_symbols_read_hook() (gold-plugin.cpp:444)
==23495==    by 0x5B7946:
gold::Plugin_manager::all_symbols_read(gold::Workqueue*, gold::Task*,
gold::Input_objects*, gold::Symbol_table*, gold::Dirsearch*, gold::Mapfile*, g
old::Task_token**) (plugin.cc:359)
==23495==    by 0x5B7A73: gold::Plugin_hook::run(gold::Workqueue*)
(plugin.cc:1420)
==23495==    by 0x609C3C: gold::Workqueue::find_and_run_task(int)
(workqueue.cc:319)
==23495==    by 0x609FC9: gold::Workqueue::process(int) (workqueue.cc:495)
==23495==    by 0x4069EF: main (main.cc:252)
==23495==  Address 0x544f1f0 is 32 bytes inside a block of size 47 free'd
==23495==    at 0x4028FF8: operator delete(void*) (vg_replace_malloc.c:480)
==23495==    by 0x5D3C92C: LTOCodeGenerator::~LTOCodeGenerator()
(basic_string.h:249)
==23495==    by 0x5C6121D: lto_codegen_dispose (lto.cpp:213)
==23495==    by 0x41EE56B: all_symbols_read_hook() (gold-plugin.cpp:435)
==23495==    by 0x5B7946:
gold::Plugin_manager::all_symbols_read(gold::Workqueue*, gold::Task*,
gold::Input_objects*, gold::Symbol_table*, gold::Dirsearch*, gold::Mapfile*, g
old::Task_token**) (plugin.cc:359)
==23495==    by 0x5B7A73: gold::Plugin_hook::run(gold::Workqueue*)
(plugin.cc:1420)
==23495==    by 0x609C3C: gold::Workqueue::find_and_run_task(int)
(workqueue.cc:319)
==23495==    by 0x609FC9: gold::Workqueue::process(int) (workqueue.cc:495)
==23495==    by 0x4069EF: main (main.cc:252)

[Quuxplusone]

This might help find the problem:

Committing to https://llvm.org/svn/llvm-project/llvm/trunk ...
    M   include/llvm/Linker.h
    M   lib/LTO/LTOCodeGenerator.cpp
    M   lib/Linker/LinkModules.cpp
Committed r192778
    M   include/llvm/Linker.h
    M   lib/LTO/LTOCodeGenerator.cpp
    M   lib/Linker/LinkModules.cpp
r192778 = 051fa76ed98dd1b04e1819d21036e5bd3c8803c4 (refs/remotes/origin/master)
No changes between 5176cffb1ab3a1d015608a7c13c1d9c7ded9602f and
refs/remotes/origin/master
Resetting to the latest refs/remotes/origin/master

[Quuxplusone]

The problem seems to be that lto_codegen_dispose(code_gen) is
called a little too early.
The following patch fixes the issue for me:

diff --git a/tools/gold/gold-plugin.cpp b/tools/gold/gold-plugin.cpp
index 119631cfa7e0..16fadf9ee431 100644
--- a/tools/gold/gold-plugin.cpp
+++ b/tools/gold/gold-plugin.cpp
@@ -432,7 +432,6 @@ static ld_plugin_status all_symbols_read_hook(void) {
     (*message)(LDPL_ERROR, "Could not produce a combined object file\n");
   }

-  lto_codegen_dispose(code_gen);
   for (std::list<claimed_file>::iterator I = Modules.begin(),
          E = Modules.end(); I != E; ++I) {
     for (unsigned i = 0; i != I->syms.size(); ++i) {
@@ -444,18 +443,22 @@ static ld_plugin_status all_symbols_read_hook(void) {
   if ((*add_input_file)(objPath) != LDPS_OK) {
     (*message)(LDPL_ERROR, "Unable to add .o file to the link.");
     (*message)(LDPL_ERROR, "File left behind in: %s", objPath);
+    lto_codegen_dispose(code_gen);
     return LDPS_ERR;
   }

   if (!options::extra_library_path.empty() &&
       set_extra_library_path(options::extra_library_path.c_str()) != LDPS_OK) {
     (*message)(LDPL_ERROR, "Unable to set the extra library path.");
+    lto_codegen_dispose(code_gen);
     return LDPS_ERR;
   }

   if (options::obj_path.empty())
     Cleanup.push_back(objPath);

+  lto_codegen_dispose(code_gen);
+
   return LDPS_OK;
 }

[Quuxplusone]

I will take a look. I assume this reproduces with any binary, correct?

[Quuxplusone]

(In reply to comment #3)
> I will take a look. I assume this reproduces with any binary, correct?

Yes.
Thanks.

[Quuxplusone]

Fixed in r192787.

I went with copying the path to a std::string instead of delaying the call to lto_codegen_dispose.

Thanks for the bug report and patch!