This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2 6/6] Libsanitizer merge from upstream r250806 (was r249633).


On 10/20/2015 02:29 PM, Maxim Ostapenko wrote:
In this patch, I'm trying to add a general instruction how to perform
the merge. This is just a documentation patch, any suggestions and
opinions are welcome.

Thanks, this should simplify work for other maintainers in future)

Some general remarks:
1) Perhaps use standard markup format for easier reading (i.e. s/^-/*/)?
2) We should suggest to run libabigail to compare ABI of libclang_rt-asan and libasan? 3) Perhaps it makes sense to mention that patchset should be split in logical pieces?
4) You probably forgot to mention SONAME update.

+- Modify Makefile.am files into asan/tsan/lsan/ubsan/sanitizer_common/interception + directories if needed. In particular, you may need to add new source files + and remove old ones in source files list, add new flags to {C, CXX}FLAGS if
+  needed and update DEFS with new defined variables.

1) Could you mention where to look for updates (CMakeLists.txt, etc.).
2) Shouldn't we rerun automake (to update Makefile.in and stuff)?
3) Also add new target platforms (if any).

+- Apply all necessary compiler changes. Be especially careful here, you must
+  not break ABI between compiler and library.

Perhaps mention that for compiler changes one should check commit history of e.g. llvm/test/Instrumentation?

+- Remove unused (deleted by merge) files from all source and include
+  directories.

This isn't clear. Doesn't merge.sh handle this?

+- Regenerate configure script and all Makefiles by autoreconf. You should use
+  exactly the same autotools version as for other GCC directories (current
+ version is 2.64, https://www.gnu.org/software/automake/faq/autotools-faq.html
+  for details how to install/use it).

Rather than stating explicit version of autotools, perhaps tell where to find the current one (e.g. it's written at start of current libsanitizer/{Makefile.in,configure}?

+- Run regression testing on at least three platforms (e.g. x86-linux-gnu,
+  x86_64-linux-gnu, aarch64-linux-gnu).

Perhaps ARM as well? We saw a number of platform-specific bugs there.

Best regards,
Yury Gribov


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]