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

-Wshift-count-negative in drivers/net/ethernet/sfc/ #1439

Closed
nathanchance opened this issue Aug 18, 2021 · 3 comments
Closed

-Wshift-count-negative in drivers/net/ethernet/sfc/ #1439

nathanchance opened this issue Aug 18, 2021 · 3 comments
Assignees
Labels
-Wshift-count-negative [ARCH] powerpc This bug impacts ARCH=powerpc asm goto related to the implementation of asm goto [BUG] llvm A bug that should be fixed in upstream LLVM [FIXED][LLVM] 14 This bug was fixed in LLVM 14.x

Comments

@nathanchance
Copy link
Member

After commit 1e688dd2a3d6 ("powerpc/bug: Provide better flexibility to WARN_ON/__WARN_FLAGS() with asm goto") in the powerpc tree, I see the following warnings with Debian's and SUSE's configs:

$ curl -LSso .config https://github.com/openSUSE/kernel-source/raw/master/config/ppc64le/default

# Remove pahole requirement
$ scripts/config -d BPF_PRELOAD -d DEBUG_INFO_BTF

$ make -skj"$(nproc)" ARCH=powerpc CC=clang CROSS_COMPILE=powerpc64le-linux-gnu- LLVM_IAS=0 olddefconfig drivers/net/ethernet/sfc/
drivers/net/ethernet/sfc/farch.c:985:10: warning: shift count is negative [-Wshift-count-negative]
        WARN_ON(EFX_QWORD_FIELD(*event, FSF_AZ_RX_EV_Q_LABEL) !=
        ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/sfc/bitfield.h:222:26: note: expanded from macro 'EFX_QWORD_FIELD'
#define EFX_QWORD_FIELD         EFX_QWORD_FIELD64
                                ^
drivers/net/ethernet/sfc/bitfield.h:173:2: note: expanded from macro 'EFX_QWORD_FIELD64'
        EFX_EXTRACT_QWORD64(qword, EFX_LOW_BIT(field),          \
        ^
drivers/net/ethernet/sfc/bitfield.h:149:3: note: expanded from macro 'EFX_EXTRACT_QWORD64'
        (EFX_EXTRACT64((qword).u64[0], 0, 63, low, high) &              \
         ^
drivers/net/ethernet/sfc/bitfield.h:134:2: note: expanded from macro 'EFX_EXTRACT64'
        EFX_EXTRACT_NATIVE(le64_to_cpu(element), min, max, low, high)
        ^
drivers/net/ethernet/sfc/bitfield.h:127:20: note: expanded from macro 'EFX_EXTRACT_NATIVE'
         (native_element) << ((min) - (low)))
                          ^
arch/powerpc/include/asm/bug.h:122:30: note: expanded from macro 'WARN_ON'
                                   __label_warn_on, "r" (x));   \
                                   ~~~~~~~~~~~~~~~~~~~~~~^~~
arch/powerpc/include/asm/bug.h:75:7: note: expanded from macro 'WARN_ENTRY'
                  ##__VA_ARGS__ : : label)
                  ~~^~~~~~~~~~~~~~~~~~~~~~
include/linux/compiler_types.h:254:42: note: expanded from macro 'asm_volatile_goto'
#define asm_volatile_goto(x...) asm goto(x)
                                         ^
1 warning generated.
drivers/net/ethernet/sfc/falcon/farch.c:994:10: warning: shift count is negative [-Wshift-count-negative]
        WARN_ON(EF4_QWORD_FIELD(*event, FSF_AZ_RX_EV_Q_LABEL) !=
        ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/sfc/falcon/bitfield.h:222:26: note: expanded from macro 'EF4_QWORD_FIELD'
#define EF4_QWORD_FIELD         EF4_QWORD_FIELD64
                                ^
drivers/net/ethernet/sfc/falcon/bitfield.h:173:2: note: expanded from macro 'EF4_QWORD_FIELD64'
        EF4_EXTRACT_QWORD64(qword, EF4_LOW_BIT(field),          \
        ^
drivers/net/ethernet/sfc/falcon/bitfield.h:149:3: note: expanded from macro 'EF4_EXTRACT_QWORD64'
        (EF4_EXTRACT64((qword).u64[0], 0, 63, low, high) &              \
         ^
drivers/net/ethernet/sfc/falcon/bitfield.h:134:2: note: expanded from macro 'EF4_EXTRACT64'
        EF4_EXTRACT_NATIVE(le64_to_cpu(element), min, max, low, high)
        ^
drivers/net/ethernet/sfc/falcon/bitfield.h:127:20: note: expanded from macro 'EF4_EXTRACT_NATIVE'
         (native_element) << ((min) - (low)))
                          ^
arch/powerpc/include/asm/bug.h:122:30: note: expanded from macro 'WARN_ON'
                                   __label_warn_on, "r" (x));   \
                                   ~~~~~~~~~~~~~~~~~~~~~~^~~
arch/powerpc/include/asm/bug.h:75:7: note: expanded from macro 'WARN_ENTRY'
                  ##__VA_ARGS__ : : label)
                  ~~^~~~~~~~~~~~~~~~~~~~~~
include/linux/compiler_types.h:254:42: note: expanded from macro 'asm_volatile_goto'
#define asm_volatile_goto(x...) asm goto(x)
                                         ^
1 warning generated.

Doing the math, I have no idea how it thinks the shift is going to be negative... (all of the macros are in drivers/net/ethernet/sfc/falcon/bitfield.h)

EF4_QWORD_FIELD(*event, FSF_AZ_RX_EV_Q_LABEL)

EFX_QWORD_FIELD64(*event, FSF_AZ_RX_EV_Q_LABEL)

EF4_EXTRACT_QWORD64(*event, EF4_LOW_BIT(FSF_AZ_RX_EV_Q_LABEL), EF4_HIGH_BIT(FSF_AZ_RX_EV_Q_LABEL))

EF4_EXTRACT_QWORD64(*event, FSF_AZ_RX_EV_Q_LABEL_LBN, FSF_AZ_RX_EV_Q_LABEL_LBN + FSF_AZ_RX_EV_Q_LABEL_WIDTH - 1)

EF4_EXTRACT_QWORD64(*event, 32, 32 + 5 - 1)

EF4_EXTRACT_QWORD64(*event, 32, 36)

EF4_EXTRACT_QWORD64(*event, 32, 36)

EF4_EXTRACT64((*event).u64[0], 0, 63, 32, 36) & EF4_MASK64((36) + 1 - (32))

EF4_EXTRACT64((*event).u64[0], 0, 63, 32, 36) & EF4_MASK64(5)

EF4_EXTRACT64((*event).u64[0], 0, 63, 32, 36) & EF4_MASK64(5)

EF4_EXTRACT_NATIVE(le64_to_cpu((*event).u64[0]), 0, 63, 32, 36) & EF4_MASK64(5)

which finally evaluates to

#define EF4_EXTRACT_NATIVE(native_element, min, max, low, high)		\
	((low) > (max) || (high) < (min) ? 0 :				\
	 (low) > (min) ?						\
	 (native_element) >> ((low) - (min)) :				\
	 (native_element) << ((min) - (low)))

EF4_EXTRACT_NATIVE(le64_to_cpu((*event).u64[0]), 0, 63, 32, 36)
	((32) > (63) || (36) < (0) ? 0 :				\
	 (32) > (0) ?						\
	 (le64_to_cpu((*event).u64[0])) >> ((32) - (0)) :				\
	 (le64_to_cpu((*event).u64[0])) << ((0) - (32)))

Maybe it is trying to evaluate the last branch as a shift of -32? I also do not see what the commit that exposed the problem has to do with this.

@nathanchance nathanchance added [BUG] Untriaged Something isn't working [ARCH] powerpc This bug impacts ARCH=powerpc [BUG] linux-next This is an issue only seen in linux-next labels Aug 18, 2021
@nathanchance
Copy link
Member Author

Looks like that is exactly what is happening. cvise spits out:

$ cat farch.i
void ef4_farch_handle_rx_event() {
  asm goto("" : : "r"(2 ?: 0 << -2) : : __label_warn_on);
__label_warn_on:;
}

$ gcc -O2 -Wall -Wextra -c -o /dev/null farch.i

$ clang -fsyntax-only -Wall -Wextra farch.i
farch.i:2:30: warning: shift count is negative [-Wshift-count-negative]
  asm goto("" : : "r"(2 ?: 0 << -2) : : __label_warn_on);
                             ^  ~~
1 warning generated.

It seems like asm goto is messing this up:

$ cat test.c
cat test.c
int foo(void)
{
        return 1 ? 2 : 0 << -1;
}

void bar(void)
{
        asm("" :: "r"(1 ? 2 : 0 << -1));
}

void baz(void)
{
        asm goto("" :: "r"(1 ? 2 : 0 << -1) :: error);
error:;
}

$ gcc -Wall -Wextra -c -o /dev/null test.c

$ clang -Wall -Wextra -c -o /dev/null test.c
test.c:13:31: warning: shift count is negative [-Wshift-count-negative]
        asm goto("" :: "r"(1 ? 2 : 0 << -1) :: error);
                                     ^  ~~
1 warning generated.

Perhaps this is a similar issue to https://llvm.org/bz51682?

@nathanchance nathanchance added -Wshift-count-negative [BUG] llvm A bug that should be fixed in upstream LLVM asm goto related to the implementation of asm goto [BUG] linux A bug that should be fixed in the mainline kernel. and removed [BUG] Untriaged Something isn't working [BUG] linux-next This is an issue only seen in linux-next labels Dec 1, 2021
@nickdesaulniers
Copy link
Member

llvm/llvm-project#51024

@nickdesaulniers nickdesaulniers self-assigned this Dec 20, 2021
@nickdesaulniers
Copy link
Member

@nickdesaulniers nickdesaulniers added [PATCH] Submitted A patch has been submitted for review and removed [BUG] linux A bug that should be fixed in the mainline kernel. labels Dec 20, 2021
@nickdesaulniers nickdesaulniers added [FIXED][LLVM] 14 This bug was fixed in LLVM 14.x and removed [PATCH] Submitted A patch has been submitted for review labels Jan 7, 2022
github-actions bot pushed a commit to tstellar/llvm-project that referenced this issue Jan 7, 2022
When performing CFG based analyses, don't forget to check the child
statements of an asm goto, such as the expressions used for
inputs+outputs.

Fixes: llvm#51024
Fixes: ClangBuiltLinux/linux#1439

Reviewed By: void, jyknight, jyu2, efriedma

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

(cherry picked from commit 3a604fd)
tstellar pushed a commit to llvm/llvm-project that referenced this issue Jan 8, 2022
When performing CFG based analyses, don't forget to check the child
statements of an asm goto, such as the expressions used for
inputs+outputs.

Fixes: #51024
Fixes: ClangBuiltLinux/linux#1439

Reviewed By: void, jyknight, jyu2, efriedma

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

(cherry picked from commit 3a604fd)
haoNoQ pushed a commit to swiftlang/llvm-project that referenced this issue Feb 23, 2022
When performing CFG based analyses, don't forget to check the child
statements of an asm goto, such as the expressions used for
inputs+outputs.

Fixes: llvm#51024
Fixes: ClangBuiltLinux/linux#1439

Reviewed By: void, jyknight, jyu2, efriedma

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

(cherry picked from commit 3a604fd)
s194604 pushed a commit to s194604/patmos-llvm-project that referenced this issue Apr 10, 2022
When performing CFG based analyses, don't forget to check the child
statements of an asm goto, such as the expressions used for
inputs+outputs.

Fixes: llvm/llvm-project#51024
Fixes: ClangBuiltLinux/linux#1439

Reviewed By: void, jyknight, jyu2, efriedma

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

(cherry picked from commit 3a604fd)
bryanpkc pushed a commit to flang-compiler/classic-flang-llvm-project that referenced this issue Apr 20, 2022
When performing CFG based analyses, don't forget to check the child
statements of an asm goto, such as the expressions used for
inputs+outputs.

Fixes: llvm/llvm-project#51024
Fixes: ClangBuiltLinux/linux#1439

Reviewed By: void, jyknight, jyu2, efriedma

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

(cherry picked from commit 3a604fd)
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this issue Oct 7, 2022
When performing CFG based analyses, don't forget to check the child
statements of an asm goto, such as the expressions used for
inputs+outputs.

Fixes: llvm/llvm-project#51024
Fixes: ClangBuiltLinux/linux#1439

Reviewed By: void, jyknight, jyu2, efriedma

Differential Revision: https://reviews.llvm.org/D116059
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-Wshift-count-negative [ARCH] powerpc This bug impacts ARCH=powerpc asm goto related to the implementation of asm goto [BUG] llvm A bug that should be fixed in upstream LLVM [FIXED][LLVM] 14 This bug was fixed in LLVM 14.x
Projects
None yet
Development

No branches or pull requests

2 participants