Page MenuHomeFreeBSD

alexander.lochmann_tu-dortmund.de (Alexander Lochmann)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 5 2019, 12:57 PM (6 w, 2 d)

Recent Activity

Jun 11 2019

alexander.lochmann_tu-dortmund.de added a comment to D20586: Updated GCC flags for i386.
In D20586#445268, @dim wrote:

So for kern.mk, is this option needed at all? Can we simply remove it?

This is a very old addition, from rS104455, where it claimed "This reduces the size of GENERIC's text space by 73999 bytes (about 2%). The bloat is from approximately 3437 strings longer than 31 characters being padded to a 32-byte boundary".
I have no objection against removing it, but some people are very fond of obtaining a kernel image that is as small as possible. So if we can keep the option in there for base gcc 4.2.1, that would be the least controversial.

Jun 11 2019, 1:01 PM
alexander.lochmann_tu-dortmund.de updated the diff for D20586: Updated GCC flags for i386.
Jun 11 2019, 1:00 PM
alexander.lochmann_tu-dortmund.de added a comment to D20587: KCOV: Use arch-dependend data type for the PCs.

I've created https://github.com/google/syzkaller/pull/1229 to fix the bug in syzkaller.

This has now been merged. Can you try with an updated kcovtrace.

Jun 11 2019, 12:54 PM
alexander.lochmann_tu-dortmund.de updated the diff for D20587: KCOV: Use arch-dependend data type for the PCs.
Jun 11 2019, 12:50 PM
alexander.lochmann_tu-dortmund.de added a comment to D20586: Updated GCC flags for i386.
In D20586#444960, @dim wrote:

In fact, the -mno-align-long-strings option is a custom addition to our base gcc (from rS169705, quite a long time ago). Upstream gcc never supported this option.
In any case, this change is incomplete in two ways:

  • It should be dependent on the gcc version. For gcc 4.2.1, keep the old spelling, assuming that it will use base gcc. For newer gcc versions, check where the new spelling was introduced, and make it dependent on that version.
  • The option is also used in stand/i386/boot2/Makefile, supposedly to keep the binary size small. This is a very sensitive area, as boot2 can blow up with different versions of compilers for no discernible reason. Any opion change there needs to be carefully tested for size regressions. And obviously, it also needs to be gcc version dependent.

There is actually a version check in stand/i386/boot2/Makefile: .if ${COMPILER_TYPE} == "gcc" && ${COMPILER_VERSION} <= 40201
So for kern.mk, is this option needed at all? Can we simply remove it?

Jun 11 2019, 10:12 AM
alexander.lochmann_tu-dortmund.de added a comment to D20587: KCOV: Use arch-dependend data type for the PCs.

For trace_cmp(), are you sure that this has to be changed as well?
The Linux Kernel uses uint64_t here as well: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/kcov.c#n116

The different versions of sanitizer_cov_trace_cmp() handle the different data type sizes. Each version of sanitizer_cov_trace_cmp() redirects to trace_cmp().
Its arguments must therefore offer space for the largest variant of sanitizer_cov_trace_cmp() which ist sanitizer_cov_trace_cmp8().
In the other cases, there is no problem.

Jun 11 2019, 10:08 AM
alexander.lochmann_tu-dortmund.de updated the diff for D20587: KCOV: Use arch-dependend data type for the PCs.
Jun 11 2019, 10:04 AM
alexander.lochmann_tu-dortmund.de added a comment to D20587: KCOV: Use arch-dependend data type for the PCs.

As for the changes: although unsigned long is the correct way of expressing native word-size, in this case uintptr_t is a better choice for a type as it is guaranteed to be large enough to hold a pointer value.

Mhmkay. I'll switch to uintptr_t.

I believe this is what was used in early versions of the patch, but it was changed to uint64_t for compatibility with Syzkaller. @andrew can better speak to the reasons for doing so and whether or not this change would cause any issues there.

That sounds intereseting, because kcovtrace provided by syzkaller itself uses unsigned long: https://github.com/google/syzkaller/blob/master/tools/kcovtrace/kcovtrace.c#L37

Also I've only given it a brief look, but you are definitely missing one or two cases in kern_kcov.c, namely the KCOV_ELEMENT_SIZE macro and arguments to trace_cmp().

I'll fix that, too.
For trace_cmp(), are you sure that this has to be changed as well?
The Linux Kernel uses uint64_t here as well: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/kcov.c#n116

Jun 11 2019, 7:00 AM

Jun 10 2019

alexander.lochmann_tu-dortmund.de created D20587: KCOV: Use arch-dependend data type for the PCs.
Jun 10 2019, 8:56 AM
alexander.lochmann_tu-dortmund.de added reviewers for D20586: Updated GCC flags for i386: dim, imp.
Jun 10 2019, 8:39 AM
alexander.lochmann_tu-dortmund.de created D20586: Updated GCC flags for i386.
Jun 10 2019, 8:37 AM