shared vs static conflict with exported symbols

[GoogleCodeExporter]

1. hg clone https://re2.googlecode.com/hg re2
2. cd re2
3. make test

It fails with the following error message:

.....

Running dynamic binary tests.

obj/so/test/charclass_test              PASS
obj/so/test/compile_test                PASS
obj/so/test/filtered_re2_test           FAIL; output in 
obj/so/test/filtered_re2_test.log
obj/so/test/mimics_pcre_test            PASS
obj/so/test/parse_test                  PASS
obj/so/test/possible_match_test         PASS
obj/so/test/re2_test                    PASS
obj/so/test/re2_arg_test                PASS
obj/so/test/regexp_test                 PASS
obj/so/test/required_prefix_test        PASS
obj/so/test/search_test                 PASS
obj/so/test/set_test                    PASS
obj/so/test/simplify_test               PASS
obj/so/test/string_generator_test       PASS
TESTS FAILED.
make: *** [shared-test] Error 1
maxime@maxime-desktop:~/HG/re2$ more obj/so/test/filtered_re2_test.log
re2/prefilter_tree.cc:288: Compile() not called
re2/testing/filtered_re2_test.cc:35: Check failed: (0) == (v.atoms.size())
FilteredRE2Test.EmptyTest
FilteredRE2Test.SmallOrTest
Aborted (core dumped)
maxime@maxime-desktop:~/HG/re2$ 

What version of the product are you using? On what operating system?

changeset:   105:aa957b5e3374
Ubuntu 13.10 Beta

Original issue reported on code.google.com by dp.max...@gmail.com on 4 Oct 2013 at 9:10

[GoogleCodeExporter]

The same error occurs with clang version 3.4 (trunk 191182).

The clang static analyzer reports some issues with RE2 code.
Untar the archive attached and simple open index.html file in a browser to get 
these reports to look at.

Original comment by dp.max...@gmail.com on 5 Oct 2013 at 3:08

Attachments:

• scan-build-2013-10-05-115532-14992-1.tgz

[GoogleCodeExporter]

Same problem here with Redhat Enterprize linux 6.4 and re2-20130802.tgz

Original comment by ricardo...@gmail.com on 11 Oct 2013 at 7:10

[GoogleCodeExporter]

using a older version(re2-20130115.tgz) there is no problem.

Original comment by ricardo...@gmail.com on 11 Oct 2013 at 7:12

[GoogleCodeExporter]

same problem with

DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=12.04
DISTRIB_CODENAME=precise
DISTRIB_DESCRIPTION="Ubuntu 12.04.3 LTS"
NAME="Ubuntu"
VERSION="12.04.3 LTS, Precise Pangolin"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu precise (12.04.3 LTS)"
VERSION_ID="12.04"

Original comment by ebie...@gmail.com on 28 Oct 2013 at 5:15

[GoogleCodeExporter]

First source to show this failure is re2-20130712.tgz. It does not occur in 
re2-20130622.tgz or earlier.

As a result, I suspect the bug lies in this commit:
http://code.google.com/p/re2/source/detail?r=aa955087d6f5dfea2a76d9204d78793edc7
f3fd9

I did a little debugging on the failing test and it looks like v.atoms.size() 
is returning 2, not the 0 it expects.

Original comment by spo...@gmail.com on 12 Nov 2013 at 4:58

[GoogleCodeExporter]

Nope, I was wrong, its the additional symbol export in the libre2.symbols file. 
The following line was added (probably for Chrome/Chromium):

_ZNK3re211FilteredRE2*;

That seems to be too broad, and causes things to break. Switching it to just 
what Chromium needs resolves the issue:

--- re2/libre2.symbols.fix  2013-11-12 10:54:28.312119611 -0500
+++ re2/libre2.symbols  2013-11-12 10:54:43.057091010 -0500
@@ -10,7 +10,7 @@
                _ZlsRSoRKN3re211StringPieceE;
                # re2::FilteredRE2*
                _ZN3re211FilteredRE2*;
-               _ZNK3re211FilteredRE2*;
+               _ZNK3re211FilteredRE210AllMatches*;
        local:
                *;
 };

Original comment by spo...@gmail.com on 12 Nov 2013 at 3:57

[GoogleCodeExporter]

Unfortunately, doesn't solve it for me. I'm on Linux mint. I applied the fix, 
but tests still fail with the same message...

Original comment by dcvetino...@gmail.com on 19 Nov 2013 at 5:04

[GoogleCodeExporter]

> Unfortunately, doesn't solve it for me. I'm on Linux mint. I applied the fix, 
but tests still fail with the same message...

Sorry, this was not correct. All test passed after I had applied the fix from 
above. 
best

Original comment by dcvetino...@gmail.com on 19 Nov 2013 at 5:18

[GoogleCodeExporter]

Looks like it hasn't been fixed yet.

The problem is that the FLAGS_filtered_re2_min_atom_len (which the test case is 
trying to modify) hasn't been exported from the shared library, so the shared 
lib's copy cannot actually be modified by the test.

All the test cases link in both shared and static versions of the library, 
quite confusingly, so this is not at all obvious!

With your patch, you end up removing two symbols from the sharedlib:
re2::FilteredRE2::FirstMatch(re2::StringPiece const&, std::vector<int, 
std::allocator<int> > const&) const
re2::FilteredRE2::SlowFirstMatch(re2::StringPiece const&) const

By doing so, the linker now has to pull in filtered_re2.o from libre2.a, which 
it wouldn't otherwise have done (as those symbols could be resolved from the 
sharedlib).

Once that .o is pulled in, all the symbols present in it take precedence, so 
re2::FilteredRE2::Compile also calls the statically linked version, which then 
means that setting the FLAG works. But that's all basically just a complete 
accident, and means the tests aren't really testing the shared lib copy at all.

Seems to me like the sharedlib tests ought to only link against the shared lib 
(plus maybe some standalone utility files), and then ought to only try to run 
the subset of tests which can actually be run against the exported APIs.

Original comment by jykni...@google.com on 30 Nov 2013 at 4:26

[GoogleCodeExporter]

jyknight, do you know how to make that happen? 
This is all opaque to me.

Original comment by rsc@golang.org on 10 Jan 2014 at 1:36

• Changed title: shared vs static conflict with exported symbols
• Changed state: HelpWanted

[GoogleCodeExporter]

I fixed this as suggested.

Original comment by rsc@golang.org on 10 Jan 2014 at 3:22

• Changed state: Fixed

[GoogleCodeExporter]

rsc: I think adding _ZN3re231FLAGS_filtered_re2* to the symbols would have been 
a little better

Original comment by stefano.rivera on 16 Jan 2014 at 8:55