Skip to content

New instance of -Wframe-larger-than with sanitizers enabled after commit d2408c417cfa #111903

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

Open
nathanchance opened this issue Oct 10, 2024 · 2 comments

Comments

@nathanchance
Copy link
Member

I see a new instance of -Wframe-larger-than when building the Linux kernel's allmodconfig target (which enables several sanitizers) for arm64, which I bisected to commit d2408c417cfa ("[InstCombine] Canonicalize more geps with constant gep bases and constant offsets. (#110033)") (also reported on our mailing list by the Linaro toolchain test infrastructure):

$ make -skj"$(nproc)" ARCH=arm64 LLVM=1 mrproper allmodconfig drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.o
drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c:1526:12: error: stack frame size (2176) exceeds limit (2048) in 'vdec_vp9_slice_update_prob' [-Werror,-Wframe-larger-than]
 1526 | static int vdec_vp9_slice_update_prob(struct vdec_vp9_slice_instance *instance,
      |            ^
1 error generated.
...

At the parent commit 45b526afa26e ("[LV] Honor uniform-after-vectorization in setVectorizedCallDecision."), the usage is much lower.

$ make -skj"$(nproc)" ARCH=arm64 LLVM=1 KCFLAGS=-Wframe-larger-than=700 mrproper allmodconfig drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.o
drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c:1526:12: error: stack frame size (864) exceeds limit (700) in 'vdec_vp9_slice_update_prob' [-Werror,-Wframe-larger-than]
 1526 | static int vdec_vp9_slice_update_prob(struct vdec_vp9_slice_instance *instance,
      |            ^
1 error generated.
...

I reduced this file with cvise:

struct v4l2_vp9_frame_symbol_counts {
  int (*coeff[4][2][2][6][6])[];
  int *eob[][2][2][6][6][2];
} *vdec_vp9_slice_update_prob_counts_helper;
struct vdec_vp9_slice_frame_counts {
  struct {
    int band_1_5[5][6];
  } eob_branch[4][2][2];
  struct {
    int band_1_5[5][6][4];
  } coef_probs[4][2][2];
  int class0_fp;
} vdec_vp9_slice_counts_map_helper_counts;
struct {
  int frame_ctx_helper;
  struct v4l2_vp9_frame_symbol_counts counts_helper;
} *vdec_vp9_slice_update_prob_instance;
static void vdec_vp9_slice_map_counts_eob_coef(
    int i, int j, int k, struct vdec_vp9_slice_frame_counts *counts,
    struct v4l2_vp9_frame_symbol_counts *counts_helper) {
  int l, m;
  for (l = 1; l < 6; l++)
    for (m = 0; m < 6; m++) {
      counts_helper->coeff[i][j][k][l][m] = (int(*)[])counts;
      counts_helper->eob[i][j][k][l][m][0] =
          &counts->eob_branch[i][j][k].band_1_5[1][m];
      counts_helper->eob[i][j][k][l][m][1] =
          &counts->coef_probs[i][j][k].band_1_5[1][m][3];
    }
}
static void vdec_vp9_slice_counts_map_helper(
    struct v4l2_vp9_frame_symbol_counts *counts_helper) {
  int i, j, k;
  for (i = 0; i < 4; i++)
    for (j = 0; j < 2; j++)
      for (k = 0; k < 2; k++)
        vdec_vp9_slice_map_counts_eob_coef(
            i, j, k, &vdec_vp9_slice_counts_map_helper_counts, counts_helper);
}
int vdec_vp9_slice_update_prob() {
  vdec_vp9_slice_update_prob_counts_helper =
      &vdec_vp9_slice_update_prob_instance->counts_helper;
  vdec_vp9_slice_counts_map_helper(vdec_vp9_slice_update_prob_counts_helper);
  return 0;
}

which results in the following behavior with GCC 14.2.0 and clang at the revisions mentioned above.

$ aarch64-linux-gcc -O2 -Wall -c -o /dev/null vdec_vp9_req_lat_if.i -Wframe-larger-than=1 -fsanitize=bounds -fsanitize=thread
vdec_vp9_req_lat_if.i: In function 'vdec_vp9_slice_update_prob':
vdec_vp9_req_lat_if.i:45:1: warning: the frame size of 112 bytes is larger than 1 bytes [-Wframe-larger-than=]
   45 | }
      | ^

$ good-clang --target=aarch64-linux-gnu -O2 -Wall -c -o /dev/null vdec_vp9_req_lat_if.i -Wframe-larger-than=1
vdec_vp9_req_lat_if.i:40:5: warning: stack frame size (128) exceeds limit (1) in 'vdec_vp9_slice_update_prob' [-Wframe-larger-than]
   40 | int vdec_vp9_slice_update_prob() {
      |     ^
1 warning generated.

$ good-clang --target=aarch64-linux-gnu -O2 -Wall -c -o /dev/null vdec_vp9_req_lat_if.i -Wframe-larger-than=1 -fsanitize=bounds -fsanitize=thread
vdec_vp9_req_lat_if.i:40:5: warning: stack frame size (336) exceeds limit (1) in 'vdec_vp9_slice_update_prob' [-Wframe-larger-than]
   40 | int vdec_vp9_slice_update_prob() {
      |     ^
1 warning generated.

$ bad-clang --target=aarch64-linux-gnu -O2 -Wall -c -o /dev/null vdec_vp9_req_lat_if.i -Wframe-larger-than=1
vdec_vp9_req_lat_if.i:40:5: warning: stack frame size (688) exceeds limit (1) in 'vdec_vp9_slice_update_prob' [-Wframe-larger-than]
   40 | int vdec_vp9_slice_update_prob() {
      |     ^
1 warning generated.

$ bad-clang --target=aarch64-linux-gnu -O2 -Wall -c -o /dev/null vdec_vp9_req_lat_if.i -Wframe-larger-than=1 -fsanitize=bounds -fsanitize=thread
vdec_vp9_req_lat_if.i:40:5: warning: stack frame size (2176) exceeds limit (1) in 'vdec_vp9_slice_update_prob' [-Wframe-larger-than]
   40 | int vdec_vp9_slice_update_prob() {
      |     ^
1 warning generated.

This is reminiscent of a downstream issue I filed for a warning in the same code with ARCH=loongarch (ClangBuiltLinux/linux#2014), which resulted in some fixes in the LoongArch backend (0822780, 8e4b089). Perhaps AArch64 needs similar changes?

cc @davemgreen @nikic

@nathanchance
Copy link
Member Author

Tentatively marking this as an issue in the AArch64 backend (since I still have not seen this with other architectures) for visibility, if that is incorrect, feel free to adjust it.

@llvmbot
Copy link
Member

llvmbot commented Oct 30, 2024

@llvm/issue-subscribers-backend-aarch64

Author: Nathan Chancellor (nathanchance)

I see a new instance of `-Wframe-larger-than` when building the Linux kernel's `allmodconfig` target (which enables several sanitizers) for arm64, which I bisected to commit [d2408c4](https://github.com/llvm/llvm-project/commit/d2408c417cfa71f1786c909788560374eb1aca96) ("[InstCombine] Canonicalize more geps with constant gep bases and constant offsets. (#110033)") (also [reported on our mailing list](https://lore.kernel.org/llvm/[email protected]/) by the Linaro toolchain test infrastructure):
$ make -skj"$(nproc)" ARCH=arm64 LLVM=1 mrproper allmodconfig drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.o
drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c:1526:12: error: stack frame size (2176) exceeds limit (2048) in 'vdec_vp9_slice_update_prob' [-Werror,-Wframe-larger-than]
 1526 | static int vdec_vp9_slice_update_prob(struct vdec_vp9_slice_instance *instance,
      |            ^
1 error generated.
...

At the parent commit 45b526afa26e ("[LV] Honor uniform-after-vectorization in setVectorizedCallDecision."), the usage is much lower.

$ make -skj"$(nproc)" ARCH=arm64 LLVM=1 KCFLAGS=-Wframe-larger-than=700 mrproper allmodconfig drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.o
drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c:1526:12: error: stack frame size (864) exceeds limit (700) in 'vdec_vp9_slice_update_prob' [-Werror,-Wframe-larger-than]
 1526 | static int vdec_vp9_slice_update_prob(struct vdec_vp9_slice_instance *instance,
      |            ^
1 error generated.
...

I reduced this file with cvise:

struct v4l2_vp9_frame_symbol_counts {
  int (*coeff[4][2][2][6][6])[];
  int *eob[][2][2][6][6][2];
} *vdec_vp9_slice_update_prob_counts_helper;
struct vdec_vp9_slice_frame_counts {
  struct {
    int band_1_5[5][6];
  } eob_branch[4][2][2];
  struct {
    int band_1_5[5][6][4];
  } coef_probs[4][2][2];
  int class0_fp;
} vdec_vp9_slice_counts_map_helper_counts;
struct {
  int frame_ctx_helper;
  struct v4l2_vp9_frame_symbol_counts counts_helper;
} *vdec_vp9_slice_update_prob_instance;
static void vdec_vp9_slice_map_counts_eob_coef(
    int i, int j, int k, struct vdec_vp9_slice_frame_counts *counts,
    struct v4l2_vp9_frame_symbol_counts *counts_helper) {
  int l, m;
  for (l = 1; l &lt; 6; l++)
    for (m = 0; m &lt; 6; m++) {
      counts_helper-&gt;coeff[i][j][k][l][m] = (int(*)[])counts;
      counts_helper-&gt;eob[i][j][k][l][m][0] =
          &amp;counts-&gt;eob_branch[i][j][k].band_1_5[1][m];
      counts_helper-&gt;eob[i][j][k][l][m][1] =
          &amp;counts-&gt;coef_probs[i][j][k].band_1_5[1][m][3];
    }
}
static void vdec_vp9_slice_counts_map_helper(
    struct v4l2_vp9_frame_symbol_counts *counts_helper) {
  int i, j, k;
  for (i = 0; i &lt; 4; i++)
    for (j = 0; j &lt; 2; j++)
      for (k = 0; k &lt; 2; k++)
        vdec_vp9_slice_map_counts_eob_coef(
            i, j, k, &amp;vdec_vp9_slice_counts_map_helper_counts, counts_helper);
}
int vdec_vp9_slice_update_prob() {
  vdec_vp9_slice_update_prob_counts_helper =
      &amp;vdec_vp9_slice_update_prob_instance-&gt;counts_helper;
  vdec_vp9_slice_counts_map_helper(vdec_vp9_slice_update_prob_counts_helper);
  return 0;
}

which results in the following behavior with GCC 14.2.0 and clang at the revisions mentioned above.

$ aarch64-linux-gcc -O2 -Wall -c -o /dev/null vdec_vp9_req_lat_if.i -Wframe-larger-than=1 -fsanitize=bounds -fsanitize=thread
vdec_vp9_req_lat_if.i: In function 'vdec_vp9_slice_update_prob':
vdec_vp9_req_lat_if.i:45:1: warning: the frame size of 112 bytes is larger than 1 bytes [-Wframe-larger-than=]
   45 | }
      | ^

$ good-clang --target=aarch64-linux-gnu -O2 -Wall -c -o /dev/null vdec_vp9_req_lat_if.i -Wframe-larger-than=1
vdec_vp9_req_lat_if.i:40:5: warning: stack frame size (128) exceeds limit (1) in 'vdec_vp9_slice_update_prob' [-Wframe-larger-than]
   40 | int vdec_vp9_slice_update_prob() {
      |     ^
1 warning generated.

$ good-clang --target=aarch64-linux-gnu -O2 -Wall -c -o /dev/null vdec_vp9_req_lat_if.i -Wframe-larger-than=1 -fsanitize=bounds -fsanitize=thread
vdec_vp9_req_lat_if.i:40:5: warning: stack frame size (336) exceeds limit (1) in 'vdec_vp9_slice_update_prob' [-Wframe-larger-than]
   40 | int vdec_vp9_slice_update_prob() {
      |     ^
1 warning generated.

$ bad-clang --target=aarch64-linux-gnu -O2 -Wall -c -o /dev/null vdec_vp9_req_lat_if.i -Wframe-larger-than=1
vdec_vp9_req_lat_if.i:40:5: warning: stack frame size (688) exceeds limit (1) in 'vdec_vp9_slice_update_prob' [-Wframe-larger-than]
   40 | int vdec_vp9_slice_update_prob() {
      |     ^
1 warning generated.

$ bad-clang --target=aarch64-linux-gnu -O2 -Wall -c -o /dev/null vdec_vp9_req_lat_if.i -Wframe-larger-than=1 -fsanitize=bounds -fsanitize=thread
vdec_vp9_req_lat_if.i:40:5: warning: stack frame size (2176) exceeds limit (1) in 'vdec_vp9_slice_update_prob' [-Wframe-larger-than]
   40 | int vdec_vp9_slice_update_prob() {
      |     ^
1 warning generated.

This is reminiscent of a downstream issue I filed for a warning in the same code with ARCH=loongarch (ClangBuiltLinux/linux#2014), which resulted in some fixes in the LoongArch backend (0822780, 8e4b089). Perhaps AArch64 needs similar changes?

cc @davemgreen @nikic

nathanchance added a commit to nathanchance/continuous-integration2 that referenced this issue Nov 18, 2024
As there has been no movement on the upstream report [1], apply Arnd's
workaround, which I have requested to be applied [2].

Link: llvm/llvm-project#111903 [1]
Link: https://lore.kernel.org/20241118200641.GA768549@thelio-3990X/ [2]
Signed-off-by: Nathan Chancellor <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants