| Message ID | CA+4CFy7jQJTcZ4eD=a-U3e6Kug5QuhMT9B5_6YrQAOx7K6sLHg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
And this is the second part. Thanks, Wei. On Fri, Oct 19, 2012 at 6:41 PM, Wei Mi <wmi@google.com> wrote: > Thanks David. Here is the patch after removing m4 directory under libasan. > And I add the dependence libasan on libstdc++-v3 to avoid problem > under parallel making for now. > > The patch is too big even after compressed, so I have to split the > patch into two parts contained in two mails. patch.part1.txt and > patch.part2.txt. This is the first part. > > --- a/Makefile.def 2012-10-19 15:36:36.156106282 -0700 > +++ b/Makefile.def 2012-10-19 15:36:51.656186869 -0700 > @@ -504,7 +504,6 @@ > dependencies = { module=configure-target-libobjc; > on=configure-target-boehm-gc; }; > dependencies = { module=all-target-libobjc; on=all-target-boehm-gc; }; > dependencies = { module=configure-target-libstdc++-v3; > on=configure-target-libgomp; }; > +dependencies = { module=configure-target-libasan; > on=all-target-libstdc++-v3; }; > // parallel_list.o and parallel_settings.o depend on omp.h, which is > // generated by the libgomp configure. Unfortunately, due to the use of > // recursive make, we can't be that specific. > > Thanks, > Wei. > > On Fri, Oct 19, 2012 at 12:52 PM, Xinliang David Li <davidxl@google.com> wrote: >> The library builds fine with the following diff. The file acinclude.m4 >> is cloned from libmudflap. >> >> David >> >> index 485d169..3e847f1 100644 >> --- a/libasan/aclocal.m4 >> +++ b/libasan/aclocal.m4 >> @@ -1037,8 +1037,8 @@ AC_SUBST([am__tar]) >> AC_SUBST([am__untar]) >> ]) # _AM_PROG_TAR >> >> -m4_include([m4/libtool.m4]) >> -m4_include([m4/ltoptions.m4]) >> -m4_include([m4/ltsugar.m4]) >> -m4_include([m4/ltversion.m4]) >> -m4_include([m4/lt~obsolete.m4]) >> +m4_include([../config/ltoptions.m4]) >> +m4_include([../config/ltsugar.m4]) >> +m4_include([../config/ltversion.m4]) >> +#m4_include([../config/lt~obsolete.m4]) >> +m4_include([acinclude.m4]) >> >> >> On Fri, Oct 19, 2012 at 11:02 AM, Wei Mi <wmi@google.com> wrote: >>> David, I put the m4 subdir under libasan because once I use the .m4 >>> files (libtool.m4 lt~obsolete.m4 ltoptions.m4 ltsugar.m4 >>> ltversion.m4) and ltmain.sh under $topsrcdir, the problem that a bad >>> libtool was generated under >>> $topbuilddir/x86_64-unknown-linux-gnu/libasan you met yesterday >>> appeared. That is why I had to generate the new libtool m4 files and >>> ltmain.sh using libtoolize. >>> >>> Thanks, >>> Wei. >>> >>> On Fri, Oct 19, 2012 at 10:16 AM, Xinliang David Li <davidxl@google.com> wrote: >>>> I tried it, and this version works for me. >>>> >>>> Your probably do not need to add the m4 subdir under libasan. The >>>> required m4 files are either in .. or ../config dir. See how >>>> libmudflap does it. >>>> >>>> Other than that, if there are no other comments, the change is good to >>>> check into the branch. Remaining bugs can always be found and fixed >>>> later. >>>> >>>> thanks, >>>> >>>> David >>>> >>>> >>>> >>>> On Thu, Oct 18, 2012 at 8:04 PM, Wei Mi <wmi@google.com> wrote: >>>>> Hi, >>>>> >>>>> David cought a problem in the last patch when he tries to built >>>>> libasan. The problem was that an incomplete libtool under libasan >>>>> build directory was generated. The cause is that the old patch used an >>>>> old ltmain.sh to generate libtool. I fix it and attach a new patch. >>>>> And the new patch move -lpthread and -ldl to libasan LDFLAGS. >>>>> >>>>> Thanks, >>>>> Wei.
To accommodate tsan (and msan in the future), the directory structure needs to be changed. The top level directory can be called something like librt with the following subdirs: asan, tsan, sanitizer_common, and interception. Also the libasan should be linked in statically by default. thanks, David On Fri, Oct 19, 2012 at 6:42 PM, Wei Mi <wmi@google.com> wrote: > And this is the second part. > > Thanks, > Wei. > > On Fri, Oct 19, 2012 at 6:41 PM, Wei Mi <wmi@google.com> wrote: >> Thanks David. Here is the patch after removing m4 directory under libasan. >> And I add the dependence libasan on libstdc++-v3 to avoid problem >> under parallel making for now. >> >> The patch is too big even after compressed, so I have to split the >> patch into two parts contained in two mails. patch.part1.txt and >> patch.part2.txt. This is the first part. >> >> --- a/Makefile.def 2012-10-19 15:36:36.156106282 -0700 >> +++ b/Makefile.def 2012-10-19 15:36:51.656186869 -0700 >> @@ -504,7 +504,6 @@ >> dependencies = { module=configure-target-libobjc; >> on=configure-target-boehm-gc; }; >> dependencies = { module=all-target-libobjc; on=all-target-boehm-gc; }; >> dependencies = { module=configure-target-libstdc++-v3; >> on=configure-target-libgomp; }; >> +dependencies = { module=configure-target-libasan; >> on=all-target-libstdc++-v3; }; >> // parallel_list.o and parallel_settings.o depend on omp.h, which is >> // generated by the libgomp configure. Unfortunately, due to the use of >> // recursive make, we can't be that specific. >> >> Thanks, >> Wei. >> >> On Fri, Oct 19, 2012 at 12:52 PM, Xinliang David Li <davidxl@google.com> wrote: >>> The library builds fine with the following diff. The file acinclude.m4 >>> is cloned from libmudflap. >>> >>> David >>> >>> index 485d169..3e847f1 100644 >>> --- a/libasan/aclocal.m4 >>> +++ b/libasan/aclocal.m4 >>> @@ -1037,8 +1037,8 @@ AC_SUBST([am__tar]) >>> AC_SUBST([am__untar]) >>> ]) # _AM_PROG_TAR >>> >>> -m4_include([m4/libtool.m4]) >>> -m4_include([m4/ltoptions.m4]) >>> -m4_include([m4/ltsugar.m4]) >>> -m4_include([m4/ltversion.m4]) >>> -m4_include([m4/lt~obsolete.m4]) >>> +m4_include([../config/ltoptions.m4]) >>> +m4_include([../config/ltsugar.m4]) >>> +m4_include([../config/ltversion.m4]) >>> +#m4_include([../config/lt~obsolete.m4]) >>> +m4_include([acinclude.m4]) >>> >>> >>> On Fri, Oct 19, 2012 at 11:02 AM, Wei Mi <wmi@google.com> wrote: >>>> David, I put the m4 subdir under libasan because once I use the .m4 >>>> files (libtool.m4 lt~obsolete.m4 ltoptions.m4 ltsugar.m4 >>>> ltversion.m4) and ltmain.sh under $topsrcdir, the problem that a bad >>>> libtool was generated under >>>> $topbuilddir/x86_64-unknown-linux-gnu/libasan you met yesterday >>>> appeared. That is why I had to generate the new libtool m4 files and >>>> ltmain.sh using libtoolize. >>>> >>>> Thanks, >>>> Wei. >>>> >>>> On Fri, Oct 19, 2012 at 10:16 AM, Xinliang David Li <davidxl@google.com> wrote: >>>>> I tried it, and this version works for me. >>>>> >>>>> Your probably do not need to add the m4 subdir under libasan. The >>>>> required m4 files are either in .. or ../config dir. See how >>>>> libmudflap does it. >>>>> >>>>> Other than that, if there are no other comments, the change is good to >>>>> check into the branch. Remaining bugs can always be found and fixed >>>>> later. >>>>> >>>>> thanks, >>>>> >>>>> David >>>>> >>>>> >>>>> >>>>> On Thu, Oct 18, 2012 at 8:04 PM, Wei Mi <wmi@google.com> wrote: >>>>>> Hi, >>>>>> >>>>>> David cought a problem in the last patch when he tries to built >>>>>> libasan. The problem was that an incomplete libtool under libasan >>>>>> build directory was generated. The cause is that the old patch used an >>>>>> old ltmain.sh to generate libtool. I fix it and attach a new patch. >>>>>> And the new patch move -lpthread and -ldl to libasan LDFLAGS. >>>>>> >>>>>> Thanks, >>>>>> Wei.
On Thu, Oct 25, 2012 at 09:24:51AM -0700, Xinliang David Li wrote: > To accommodate tsan (and msan in the future), the directory structure > needs to be changed. The top level directory can be called something > like librt with the following subdirs: asan, tsan, sanitizer_common, > and interception. Also the libasan should be linked in statically by > default. librt is a very bad name, that clashes with glibc librt, would only create confusion. Why should be libasan linked statically by default? Jakub
On Thu, Oct 25, 2012 at 9:39 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Thu, Oct 25, 2012 at 09:24:51AM -0700, Xinliang David Li wrote: >> To accommodate tsan (and msan in the future), the directory structure >> needs to be changed. The top level directory can be called something >> like librt with the following subdirs: asan, tsan, sanitizer_common, >> and interception. Also the libasan should be linked in statically by >> default. > > librt is a very bad name, that clashes with glibc librt, would only create > confusion. Ok, then we should pick something that is not confusing but reflect the fact they are for runtime error checking .. > > Why should be libasan linked statically by default? > There are a couple of reasons: 1) it makes running sanitized binary on remote machines which does not have libasan installed easier; 2) There is no guarantee that libasan API won't change, statically linking it in makes it less vulnerable to such changes. thanks, David > Jakub
On Thu, Oct 25, 2012 at 12:48 PM, Xinliang David Li <davidxl@google.com> wrote: > On Thu, Oct 25, 2012 at 9:39 AM, Jakub Jelinek <jakub@redhat.com> wrote: >> >> librt is a very bad name, that clashes with glibc librt, would only create >> confusion. > > Ok, then we should pick something that is not confusing but reflect > the fact they are for runtime error checking .. libsanitizer? Diego.
On Thu, Oct 25, 2012 at 9:52 AM, Diego Novillo <dnovillo@google.com> wrote: > On Thu, Oct 25, 2012 at 12:48 PM, Xinliang David Li <davidxl@google.com> wrote: >> On Thu, Oct 25, 2012 at 9:39 AM, Jakub Jelinek <jakub@redhat.com> wrote: >>> >>> librt is a very bad name, that clashes with glibc librt, would only create >>> confusion. >> >> Ok, then we should pick something that is not confusing but reflect >> the fact they are for runtime error checking .. > > libsanitizer? +1 --- missed the obvious one :) David > > > Diego.
On Thu, Oct 25, 2012 at 09:48:54AM -0700, Xinliang David Li wrote: > > Why should be libasan linked statically by default? > There are a couple of reasons: > > 1) it makes running sanitized binary on remote machines which does not > have libasan installed easier; > 2) There is no guarantee that libasan API won't change, statically > linking it in makes it less vulnerable to such changes. Neither of the reasons look important enough, compared to the http://www.akkadia.org/drepper/no_static_linking.html Especially as libasan is overriding malloc etc., having dozens of malloc overrides in different shared libraries, perhaps all chaining into each other, is a nightmare. For 2), libasan should just use symbol versioning and provide backwards compatibility as long as it is possible, or worst case bump SONAME occassionally. Jakub
On Thu, Oct 25, 2012 at 9:55 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Thu, Oct 25, 2012 at 09:48:54AM -0700, Xinliang David Li wrote: >> > Why should be libasan linked statically by default? > >> There are a couple of reasons: >> >> 1) it makes running sanitized binary on remote machines which does not >> have libasan installed easier; >> 2) There is no guarantee that libasan API won't change, statically >> linking it in makes it less vulnerable to such changes. > > Neither of the reasons look important enough, compared to the > http://www.akkadia.org/drepper/no_static_linking.html > Especially as libasan is overriding malloc etc., having dozens of malloc > overrides in different shared libraries, perhaps all chaining into each > other, is a nightmare. How about statically linking just for executables, not shared library buid? David > For 2), libasan should just use symbol versioning and provide backwards > compatibility as long as it is possible, or worst case bump SONAME > occassionally. > > Jakub
On Thu, Oct 25, 2012 at 10:00:03AM -0700, Xinliang David Li wrote:
> How about statically linking just for executables, not shared library buid?
That is IMHO still a bad idea.
Jakub
On Thu, Oct 25, 2012 at 10:19 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Thu, Oct 25, 2012 at 10:00:03AM -0700, Xinliang David Li wrote: >> How about statically linking just for executables, not shared library buid? > > That is IMHO still a bad idea. I don't know why you think so (It seems that the points mentioned in http://www.akkadia.org/drepper/no_static_linking.html mainly apply to release binaries, not sanitized ones), but for now let's drop the static link request. thanks, David > > Jakub
On Thu, Oct 25, 2012 at 1:24 PM, Xinliang David Li <davidxl@google.com> wrote: > I don't know why you think so (It seems that the points mentioned in > http://www.akkadia.org/drepper/no_static_linking.html mainly apply to > release binaries, not sanitized ones), but for now let's drop the > static link request. Yeah, we can always just do that in our own builds. Diego.
--- a/Makefile.def 2012-10-19 15:36:36.156106282 -0700 +++ b/Makefile.def 2012-10-19 15:36:51.656186869 -0700 @@ -504,7 +504,6 @@ dependencies = { module=configure-target-libobjc; on=configure-target-boehm-gc; }; dependencies = { module=all-target-libobjc; on=all-target-boehm-gc; }; dependencies = { module=configure-target-libstdc++-v3; on=configure-target-libgomp; }; +dependencies = { module=configure-target-libasan; on=all-target-libstdc++-v3; }; // parallel_list.o and parallel_settings.o depend on omp.h, which is