diff mbox series

[v4] kconfig: recursive checks drop file/lineno

Message ID 20240717015042.339931-1-elsk@google.com (mailing list archive)
State New
Headers show
Series [v4] kconfig: recursive checks drop file/lineno | expand

Commit Message

HONG Yifan July 17, 2024, 1:50 a.m. UTC
This prevents segfault when getting filename and lineno in recursive
checks.

If the following snippet is found in Kconfig:

[Test code 1]

config FOO
        bool
        depends on BAR
        select BAR

... without BAR defined; then there is a segfault.

  Kconfig:34:error: recursive dependency detected!
  Kconfig:34:	symbol FOO depends on BAR
  make[4]: *** [scripts/kconfig/Makefile:85: allnoconfig] Segmentation fault

This is because of the following. BAR is a fake entry created by
sym_lookup() with prop being NULL. In the recursive check, there is a
NULL check for prop to fall back to stack->sym->prop if stack->prop is
NULL. However, in this case, stack->sym points to the fake BAR entry
created by sym_lookup(), so prop is still NULL. prop was then referenced
without additional NULL checks, causing segfault.

As the previous email thread suggests, the file and lineno for select is
also wrong:

[Test code 2]

config FOO
       bool

config BAR
       bool

config FOO
       bool "FOO"
       depends on BAR
       select BAR

  $ make defconfig
  *** Default configuration is based on 'x86_64_defconfig'
  Kconfig:1:error: recursive dependency detected!
  Kconfig:1: symbol FOO depends on BAR
  Kconfig:4: symbol BAR is selected by FOO
  [...]

Kconfig:4 should be Kconfig:10.

This patch deletes the wrong and segfault-prone filename/lineno
inference completely. With this patch, Test code 1 yields:

error: recursive dependency detected!
	symbol FOO depends on BAR
	symbol BAR is selected by FOO

Link: https://lore.kernel.org/linux-kbuild/20240627231919.2461945-1-elsk@google.com/T/
Link: https://lore.kernel.org/linux-kbuild/20240620211112.500465-1-elsk@google.com/
Link: https://lore.kernel.org/linux-kbuild/20240618185609.4096399-1-elsk@google.com/
Signed-off-by: HONG Yifan <elsk@google.com>

--
v4: Rebased & addressed comments from masahiroy@kernel.org. Simplify
    code by dropping `prop` variable.
v3: Rebase on top of
    https://lore.kernel.org/linux-kbuild/20240626182212.3758235-1-masahiroy@kernel.org/T/#t
    & resolve merge conflicts. Fix
    scripts/kconfig/tests/err_recursive_dep/expected_stderr
v2: Delete all filenames/lineno completely as suggested by
    masahiroy@kernel.org
---
 scripts/kconfig/symbol.c                      | 42 ++++++-------------
 .../tests/err_recursive_dep/expected_stderr   | 36 ++++++++--------
 2 files changed, 30 insertions(+), 48 deletions(-)

Comments

Masahiro Yamada July 17, 2024, 5:10 p.m. UTC | #1
On Wed, Jul 17, 2024 at 10:51 AM HONG Yifan <elsk@google.com> wrote:
>
> This prevents segfault when getting filename and lineno in recursive
> checks.
>
> If the following snippet is found in Kconfig:
>
> [Test code 1]
>
> config FOO
>         bool
>         depends on BAR
>         select BAR
>
> ... without BAR defined; then there is a segfault.
>
>   Kconfig:34:error: recursive dependency detected!
>   Kconfig:34:   symbol FOO depends on BAR
>   make[4]: *** [scripts/kconfig/Makefile:85: allnoconfig] Segmentation fault
>
> This is because of the following. BAR is a fake entry created by
> sym_lookup() with prop being NULL. In the recursive check, there is a
> NULL check for prop to fall back to stack->sym->prop if stack->prop is
> NULL. However, in this case, stack->sym points to the fake BAR entry
> created by sym_lookup(), so prop is still NULL. prop was then referenced
> without additional NULL checks, causing segfault.
>
> As the previous email thread suggests, the file and lineno for select is
> also wrong:
>
> [Test code 2]
>
> config FOO
>        bool
>
> config BAR
>        bool
>
> config FOO
>        bool "FOO"
>        depends on BAR
>        select BAR
>
>   $ make defconfig
>   *** Default configuration is based on 'x86_64_defconfig'
>   Kconfig:1:error: recursive dependency detected!
>   Kconfig:1: symbol FOO depends on BAR
>   Kconfig:4: symbol BAR is selected by FOO
>   [...]
>
> Kconfig:4 should be Kconfig:10.
>
> This patch deletes the wrong and segfault-prone filename/lineno
> inference completely. With this patch, Test code 1 yields:
>
> error: recursive dependency detected!
>         symbol FOO depends on BAR
>         symbol BAR is selected by FOO
>
> Link: https://lore.kernel.org/linux-kbuild/20240627231919.2461945-1-elsk@google.com/T/
> Link: https://lore.kernel.org/linux-kbuild/20240620211112.500465-1-elsk@google.com/
> Link: https://lore.kernel.org/linux-kbuild/20240618185609.4096399-1-elsk@google.com/
> Signed-off-by: HONG Yifan <elsk@google.com>
>
> --
> v4: Rebased & addressed comments from masahiroy@kernel.org. Simplify
>     code by dropping `prop` variable.
> v3: Rebase on top of
>     https://lore.kernel.org/linux-kbuild/20240626182212.3758235-1-masahiroy@kernel.org/T/#t
>     & resolve merge conflicts. Fix
>     scripts/kconfig/tests/err_recursive_dep/expected_stderr
> v2: Delete all filenames/lineno completely as suggested by
>     masahiroy@kernel.org


Applied to linux-kbuild.
Thanks!
diff mbox series

Patch

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 6c6f238c4f7b..71502abd3b12 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -1074,10 +1074,9 @@  static void sym_check_print_recursive(struct symbol *last_sym)
 {
 	struct dep_stack *stack;
 	struct symbol *sym, *next_sym;
-	struct menu *menu = NULL;
 	struct menu *choice;
-	struct property *prop;
 	struct dep_stack cv_stack;
+	enum prop_type type;
 
 	choice = sym_get_choice_menu(last_sym);
 	if (choice) {
@@ -1096,53 +1095,36 @@  static void sym_check_print_recursive(struct symbol *last_sym)
 	for (; stack; stack = stack->next) {
 		sym = stack->sym;
 		next_sym = stack->next ? stack->next->sym : last_sym;
-		prop = stack->prop;
-		if (prop == NULL)
-			prop = stack->sym->prop;
-
-		/* for choice values find the menu entry (used below) */
-		if (sym_is_choice(sym) || sym_is_choice_value(sym)) {
-			for (prop = sym->prop; prop; prop = prop->next) {
-				menu = prop->menu;
-				if (prop->menu)
-					break;
-			}
-		}
+		type = stack->prop ? stack->prop->type : P_UNKNOWN;
+
 		if (stack->sym == last_sym)
-			fprintf(stderr, "%s:%d:error: recursive dependency detected!\n",
-				prop->filename, prop->lineno);
+			fprintf(stderr, "error: recursive dependency detected!\n");
 
 		if (sym_is_choice(next_sym)) {
 			choice = list_first_entry(&next_sym->menus, struct menu, link);
 
-			fprintf(stderr, "%s:%d:\tsymbol %s is part of choice block at %s:%d\n",
-				menu->filename, menu->lineno,
+			fprintf(stderr, "\tsymbol %s is part of choice block at %s:%d\n",
 				sym->name ? sym->name : "<choice>",
 				choice->filename, choice->lineno);
 		} else if (stack->expr == &sym->dir_dep.expr) {
-			fprintf(stderr, "%s:%d:\tsymbol %s depends on %s\n",
-				prop->filename, prop->lineno,
+			fprintf(stderr, "\tsymbol %s depends on %s\n",
 				sym->name ? sym->name : "<choice>",
 				next_sym->name);
 		} else if (stack->expr == &sym->rev_dep.expr) {
-			fprintf(stderr, "%s:%d:\tsymbol %s is selected by %s\n",
-				prop->filename, prop->lineno,
+			fprintf(stderr, "\tsymbol %s is selected by %s\n",
 				sym->name, next_sym->name);
 		} else if (stack->expr == &sym->implied.expr) {
-			fprintf(stderr, "%s:%d:\tsymbol %s is implied by %s\n",
-				prop->filename, prop->lineno,
+			fprintf(stderr, "\tsymbol %s is implied by %s\n",
 				sym->name, next_sym->name);
 		} else if (stack->expr) {
-			fprintf(stderr, "%s:%d:\tsymbol %s %s value contains %s\n",
-				prop->filename, prop->lineno,
+			fprintf(stderr, "\tsymbol %s %s value contains %s\n",
 				sym->name ? sym->name : "<choice>",
-				prop_get_type_name(prop->type),
+				prop_get_type_name(type),
 				next_sym->name);
 		} else {
-			fprintf(stderr, "%s:%d:\tsymbol %s %s is visible depending on %s\n",
-				prop->filename, prop->lineno,
+			fprintf(stderr, "\tsymbol %s %s is visible depending on %s\n",
 				sym->name ? sym->name : "<choice>",
-				prop_get_type_name(prop->type),
+				prop_get_type_name(type),
 				next_sym->name);
 		}
 	}
diff --git a/scripts/kconfig/tests/err_recursive_dep/expected_stderr b/scripts/kconfig/tests/err_recursive_dep/expected_stderr
index 05d4ced70320..fc2e860af082 100644
--- a/scripts/kconfig/tests/err_recursive_dep/expected_stderr
+++ b/scripts/kconfig/tests/err_recursive_dep/expected_stderr
@@ -1,38 +1,38 @@ 
-Kconfig:5:error: recursive dependency detected!
-Kconfig:5:	symbol A depends on A
+error: recursive dependency detected!
+	symbol A depends on A
 For a resolution refer to Documentation/kbuild/kconfig-language.rst
 subsection "Kconfig recursive dependency limitations"
 
-Kconfig:11:error: recursive dependency detected!
-Kconfig:11:	symbol B is selected by B
+error: recursive dependency detected!
+	symbol B is selected by B
 For a resolution refer to Documentation/kbuild/kconfig-language.rst
 subsection "Kconfig recursive dependency limitations"
 
-Kconfig:17:error: recursive dependency detected!
-Kconfig:17:	symbol C1 depends on C2
-Kconfig:21:	symbol C2 depends on C1
+error: recursive dependency detected!
+	symbol C1 depends on C2
+	symbol C2 depends on C1
 For a resolution refer to Documentation/kbuild/kconfig-language.rst
 subsection "Kconfig recursive dependency limitations"
 
-Kconfig:27:error: recursive dependency detected!
-Kconfig:27:	symbol D1 depends on D2
-Kconfig:32:	symbol D2 is selected by D1
+error: recursive dependency detected!
+	symbol D1 depends on D2
+	symbol D2 is selected by D1
 For a resolution refer to Documentation/kbuild/kconfig-language.rst
 subsection "Kconfig recursive dependency limitations"
 
-Kconfig:37:error: recursive dependency detected!
-Kconfig:37:	symbol E1 depends on E2
-Kconfig:42:	symbol E2 is implied by E1
+error: recursive dependency detected!
+	symbol E1 depends on E2
+	symbol E2 is implied by E1
 For a resolution refer to Documentation/kbuild/kconfig-language.rst
 subsection "Kconfig recursive dependency limitations"
 
-Kconfig:49:error: recursive dependency detected!
-Kconfig:49:	symbol F1 default value contains F2
-Kconfig:51:	symbol F2 depends on F1
+error: recursive dependency detected!
+	symbol F1 default value contains F2
+	symbol F2 depends on F1
 For a resolution refer to Documentation/kbuild/kconfig-language.rst
 subsection "Kconfig recursive dependency limitations"
 
-Kconfig:60:error: recursive dependency detected!
-Kconfig:60:	symbol G depends on G
+error: recursive dependency detected!
+	symbol G depends on G
 For a resolution refer to Documentation/kbuild/kconfig-language.rst
 subsection "Kconfig recursive dependency limitations"