Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sizeof(struct input_device_id)=164 is not a modulo of the size of section #1208

Closed
arndb opened this issue Dec 4, 2020 · 14 comments
Closed

sizeof(struct input_device_id)=164 is not a modulo of the size of section #1208

arndb opened this issue Dec 4, 2020 · 14 comments

Comments

@arndb
Copy link

@arndb arndb commented Dec 4, 2020

This happens on a few arch/arm randconfig builds with clang-12 on linux-next (currently 20201203):

FATAL: modpost: drivers/input/mousedev: sizeof(struct input_device_id)=164 is not a modulo of the size of section mod_input_device_table=1216.

Example .config: https://pastebin.com/MEDgUh6T

@arndb
Copy link
Author

@arndb arndb commented Dec 4, 2020

#1045 has a similar error, but that one is marked fixed

@nathanchance
Copy link
Member

@nathanchance nathanchance commented Dec 4, 2020

I did a bisect on this a little while ago and it appears related to KASAN.

@arndb
Copy link
Author

@arndb arndb commented Dec 7, 2020

I have a minimized test case at https://godbolt.org/z/ozzjTE

This shows the 'joydev_ids' array being padded from 100 to 160 bytes with 'clang -fsanitize=kernel-address'.

@nathanchance
Copy link
Member

@nathanchance nathanchance commented Dec 8, 2020

A bisect confirms that llvm/llvm-project@d3f8931 (v2 of the patch that caused #1045) introduced this failure:

$ git bisect log
# bad: [176249bd6732a8044d457092ed932768724a6f06] [CodeGen][TailDuplicator] Don't duplicate blocks with INLINEASM_BR
# good: [5852475e2c049ce29dcb1f0da3ac33035f8c9156] Bump the trunk major version to 11
git bisect start 'llvmorg-11.0.0' 'llvmorg-11-init'
# good: [b98ad941a40c96c841bceb171725c925500fce6c] [flang] Merge flang-compiler/f18
git bisect good b98ad941a40c96c841bceb171725c925500fce6c
# good: [77b1ed4b4a492d5236f936f14caedd44b275e472] [SVE] Eliminate calls to default-false VectorType::get() from Linker
git bisect good 77b1ed4b4a492d5236f936f14caedd44b275e472
# bad: [433c9adf7b2bea8577a15b5e8c38f2844b965be8] [SVE] Remove calls to VectorType::getNumElements from AsmParser
git bisect bad 433c9adf7b2bea8577a15b5e8c38f2844b965be8
# bad: [ea12216fad6e34c4638fcac8a9202c2eada0f95a] [gn build] Port 0ee176edc8b
git bisect bad ea12216fad6e34c4638fcac8a9202c2eada0f95a
# good: [c6aa829644f30d5590451b892918298f8117c985] [Dexter] Add DexLimitSteps command and ConditionalController
git bisect good c6aa829644f30d5590451b892918298f8117c985
# good: [4db2b70248686b8ac0667237768008762a66bb06] Add a flag to debug automatic variable initialization
git bisect good 4db2b70248686b8ac0667237768008762a66bb06
# good: [2a3f5021f5db56c0190e255fbde3fe619fa9e395] Added test case for the patch D75866 "supporting the visibility attribute for aix assembly"
git bisect good 2a3f5021f5db56c0190e255fbde3fe619fa9e395
# bad: [ea1bd95411c3a709ef1a405ffed9880351fe530e] AMDGPU/GlobalISel: Make G_IMPLICIT_DEF legality more consistent
git bisect bad ea1bd95411c3a709ef1a405ffed9880351fe530e
# good: [5a3b380f497b98d824b529a3369370629847a4ea] Revert "[InstrProfiling] Use !associated metadata for counters, data and values"
git bisect good 5a3b380f497b98d824b529a3369370629847a4ea
# good: [a0e3ceea6ce909067717bb703f4aaf84d88a3bbb] [AArch64][SVE] Change pointer type of struct load/store intrinsics.
git bisect good a0e3ceea6ce909067717bb703f4aaf84d88a3bbb
# bad: [4e3a44d42eace1924c9cba3b7c1ea9cdbbd6cb48] [clangd] Disable new errs()-tie behavior, it's racy.
git bisect bad 4e3a44d42eace1924c9cba3b7c1ea9cdbbd6cb48
# bad: [60f5b0ec7ce27e079513fdaa396acac30ab27bf2] [ELF][AArch64] Correct relocation codes for R_<CLS>_PLT32
git bisect bad 60f5b0ec7ce27e079513fdaa396acac30ab27bf2
# bad: [d3f89314ff20ce1612bd5e09f9f90ff5dd5205a7] [KernelAddressSanitizer] Make globals constructors compatible with kernel [v2]
git bisect bad d3f89314ff20ce1612bd5e09f9f90ff5dd5205a7
# good: [be44b7925722a037896ea59f2851f88d67ce14e6] [lld][test] Expand testing for dynamic-list and export-dynamic
git bisect good be44b7925722a037896ea59f2851f88d67ce14e6
# first bad commit: [d3f89314ff20ce1612bd5e09f9f90ff5dd5205a7] [KernelAddressSanitizer] Make globals constructors compatible with kernel [v2]

cc @melver @ramosian-glider

@melver
Copy link

@melver melver commented Dec 8, 2020

Clang should not add redzones to:

  • globals in sections
  • variables prefixed by __
  • variables aliased by aliases prefixed by __

But clearly, somewhere this is going wrong. Let me check.

@arndb
Copy link
Author

@arndb arndb commented Dec 8, 2020

adding a __ prefix to the variable avoids the redzone, so I guess the problem is the alias detection.

@melver
Copy link

@melver melver commented Dec 8, 2020

I can see what's going wrong: the optimizer turns an alias to a GlobalVariable (what we expected GlobalAlias::getAliasee() to return) into a ConstantExpr which is a pointer to that GlobalVariable, and therefore dyn_cast<GlobalVariable>(GA.getAliasee()) returns nullptr. See the code in ModuleAddressSanitizer::InstrumentGlobals() in llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp.

I have no idea why this would ever be a good optimization, but I guess we have to deal with that. [I'll only have time to resume work on this late tomorrow, so if anybody has spare cycles, help appreciated.]

@melver
Copy link

@melver melver commented Dec 8, 2020

https://reviews.llvm.org/D92846

Still need to write tests for that, then I'll send for review. If you can test, that'd be good. Thank you.

@nathanchance
Copy link
Member

@nathanchance nathanchance commented Dec 8, 2020

@melver I can confirm that patch fixes the issue.

melver added a commit to llvm/llvm-project that referenced this issue Dec 11, 2020
GlobalAlias::getAliasee() may not always point directly to a
GlobalVariable. In such cases, try to find the canonical GlobalVariable
that the alias refers to.

Link: ClangBuiltLinux/linux#1208

Reviewed By: dvyukov, nickdesaulniers

Differential Revision: https://reviews.llvm.org/D92846
@melver
Copy link

@melver melver commented Dec 11, 2020

This is now fixed for LLVM/Clang 12.

@melver melver closed this Dec 11, 2020
@nathanchance
Copy link
Member

@nathanchance nathanchance commented Dec 11, 2020

We should try to get that fix into LLVM 11.0.1, cc @tstellar, I can try to open an LLVM bug today if you need it.

@tstellar
Copy link

@tstellar tstellar commented Dec 11, 2020

@nathanchance It's pretty late, so it might not get in, but you can go ahead and file the bug anyway.

@nathanchance
Copy link
Member

@nathanchance nathanchance commented Dec 12, 2020

I have submitted https://bugs.llvm.org/show_bug.cgi?id=48492. If it cannot be added, this will need to be added to the kernel :(

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index b2bf019dcefa..a05653a17dbd 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -67,7 +67,8 @@ config ARM
        select HAVE_ARCH_BITREVERSE if (CPU_32v7M || CPU_32v7) && !CPU_32v6
        select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL && !CPU_ENDIAN_BE32 && MMU
        select HAVE_ARCH_KGDB if !CPU_ENDIAN_BE32 && MMU
-       select HAVE_ARCH_KASAN if MMU && !XIP_KERNEL
+       # https://github.com/ClangBuiltLinux/linux/issues/1208
+       select HAVE_ARCH_KASAN if MMU && !XIP_KERNEL && (CC_IS_GCC || (CLANG_VERSION >= 120000 || CLANG_VERSION <= 100001))
        select HAVE_ARCH_MMAP_RND_BITS if MMU
        select HAVE_ARCH_SECCOMP
        select HAVE_ARCH_SECCOMP_FILTER if AEABI && !OABI_COMPAT
github-actions bot pushed a commit to tstellar/llvm-project that referenced this issue Dec 12, 2020
GlobalAlias::getAliasee() may not always point directly to a
GlobalVariable. In such cases, try to find the canonical GlobalVariable
that the alias refers to.

Link: ClangBuiltLinux/linux#1208

Reviewed By: dvyukov, nickdesaulniers

Differential Revision: https://reviews.llvm.org/D92846

(cherry picked from commit c28b18a)
github-actions bot pushed a commit to tstellar/llvm-project that referenced this issue Dec 12, 2020
GlobalAlias::getAliasee() may not always point directly to a
GlobalVariable. In such cases, try to find the canonical GlobalVariable
that the alias refers to.

Link: ClangBuiltLinux/linux#1208

Reviewed By: dvyukov, nickdesaulniers

Differential Revision: https://reviews.llvm.org/D92846

(cherry picked from commit c28b18a)
@melver
Copy link

@melver melver commented Dec 12, 2020

I have submitted https://bugs.llvm.org/show_bug.cgi?id=48492.

Thank you!

If it cannot be added, this will need to be added to the kernel :(

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index b2bf019dcefa..a05653a17dbd 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -67,7 +67,8 @@ config ARM
        select HAVE_ARCH_BITREVERSE if (CPU_32v7M || CPU_32v7) && !CPU_32v6
        select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL && !CPU_ENDIAN_BE32 && MMU
        select HAVE_ARCH_KGDB if !CPU_ENDIAN_BE32 && MMU
-       select HAVE_ARCH_KASAN if MMU && !XIP_KERNEL
+       # https://github.com/ClangBuiltLinux/linux/issues/1208
+       select HAVE_ARCH_KASAN if MMU && !XIP_KERNEL && (CC_IS_GCC || (CLANG_VERSION >= 120000 || CLANG_VERSION <= 100001))
        select HAVE_ARCH_MMAP_RND_BITS if MMU
        select HAVE_ARCH_SECCOMP
        select HAVE_ARCH_SECCOMP_FILTER if AEABI && !OABI_COMPAT

Just FYI, this is overkill. We just need '-mllvm -asan-globals=0' as a last resort.

tstellar added a commit to tstellar/llvm-project that referenced this issue Dec 14, 2020
GlobalAlias::getAliasee() may not always point directly to a
GlobalVariable. In such cases, try to find the canonical GlobalVariable
that the alias refers to.

Link: ClangBuiltLinux/linux#1208

Reviewed By: dvyukov, nickdesaulniers

Differential Revision: https://reviews.llvm.org/D92846

(cherry picked from commit c28b18a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants