CONFIG_X86_X32_ABI is not visible (defined in Makefile, not C or Kconfig)

[CS-cwhite]

CONFIG_X86_X32_ABI is used throughout the kernel (example). However, it is not visible in Elixir (ref). I would like it to be.

I think the cause might be that this symbol is defined in a Makefile, not in a C, H, or Kconfig file (ref).

Thanks to @bginsbach for reporting!

[fstachura]

It seems that CONFIG_X86_X32_ABI is now visible in Elixir in linked version (although I'm not sure why it wasn't available earlier). However, the definition is not recognized.

This is now defined in Kconfig, so this is not a problem in later kernels, see https://elixir.bootlin.com/linux/v6.9.3/A/ident/CONFIG_X86_X32_ABI

[tleb]

Why are not all config variables not affected? On the same version (v5.14.15), CONFIG_OF has a "defined" entry.

[fstachura]

Why are not all config variables not affected? On the same version (v5.14.15), CONFIG_OF has a "defined" entry.

Because CONFIG_OF was defined in Kconfig. CONFIG_X86_X32_ABI was defined in a Makefile and it seems that Elixir does not support that.

As for why references were recorded despite CONFIG_X86_X32_ABI having no definitions:
Each value in the definitions database contains locations of definitions for a keyword in all versions of the project.
When references for an identifier are updated, an identifier is considered a reference if it exists in the definitions database. However, the script does not care whether the identifier was defined in currently processed version.
https://github.com/bootlin/elixir/blob/master/update.py#L318-L320

Normally, the index would be built chronologically, from earliest version to latest. That would suggest that if CONFIG_X86_X32_ABI was defined after v5.15.14, no references to it would be registered for v5.15.14. References to that identifier would be available starting from the first kernel that had a definition for that identifier that's recognized by ctags.
However, multiple issues suggest that the Elixir database was rebuilt a couple of times. Git seems to lists tags in order of creation, from latest to earliest, which also suggests that new tags were processed before old tags. Therefore, when v5.15.14 was being indexed, the first tagged kernel that had a definition for CONFIG_X86_X32_ABI (v5.18-rc1) was already indexed.

To confirm this, I first indexed the repository with only v5.14.15 tagged, then tagged v6.8 and ran indexing again. No references were recorded for CONFIG_X86_X32_ABI in v5.14.15.
Results can be seen here: https://elixir.fbstc.org/linux/v5.14.15/A/ident/CONFIG_X86_X32_ABI

[tleb]

This is great investigation, thanks! Questions I have with my naive understanding:

• We do not run ./script.sh parse-defs _ _ M, meaning we do not parse definitions in Makefiles. Or maybe we do this call, but parse_defs() only knows about families C, K and D. It appears like ctags supports being ran on Makefiles. Should we do it?

  ⟩ ctags --list-languages | grep -i make
  Automake
  CMake
  Make
  

• Would it make sense to only store references if a definition exists in the current version of the project? It would avoid having indexing be dependent on version ordering.

• Is there any other change you have in mind related to this topic?

[fstachura]

It appears like ctags supports being ran on Makefiles. Should we do it?

I don't think ctags will help here, export is only used to pass variables to sub-makefiles. I believe that CONFIG_X86_X32_ABI becomes visible in the kernel a few lines earlier, when it's defined from compiler flags with -D. On this particular Makefile, ctags reports CONFIG_X86_X32_ABI as a macro, along with many other identifiers that are not used in the kernel at all.
I guess we could try to extract symbols from -D flags using various heuristics, but I don't know if it's worth it. For example, other tools called from Makefiles could use -D for something else.

Would it make sense to only store references if a definition exists in the current version of the project? It would avoid having indexing be dependent on version ordering.

I'm actually not sure! On one hand, current behavior is very hacky and works only sometimes. On the other, it's probably better to have references to an identifier without a definition, than to have no references at all.

[tleb]

Ah yes, parsing Makefiles does not extract the interesting information we want. In this case, we are being lucky that the same Makefile is defining variables of the same name. This is not true for many other -D calls. See for example all -D__ that define variables only targeted at code and that therefore do not get exported to neighbor makefiles.

I'm actually not sure! On one hand, current behavior is very hacky and works only sometimes. On the other, it's probably better to have references to an identifier without a definition, than to have no references at all.

The issue is that it sounds like it would make the database massive. Indeed, we would not be able to filter the large portion of words outputted by the tokenizer.

Indexing only musl v0.5.0, I see 2961 keys in references.db. I get 12844 keys when removing the "definition must exist" filter. Also, we could grab dumb references such as DCONFIG_X86_X32_ABI.

So leads are:

• Use heuristics (ie regexps) to parse makefiles -D flags.
• Experiment with allowing references even when no definitions are involved.

Both of those sound like low-priority in that they should have an impact on a small number of identifiers.

We might want to address the issue of taking old definitions into account as that lead to unexpected indexing. It could cause us to lose many interesting references indexing however (eg what is happening to CONFIG_X86_X32_ABI or __KERNEL__ for example).

[masatake]

The heuristics look interesting to me.

the heuristics:

$ cat /tmp/make-flags.ctags 
--langdef=MyMake{base=Make}{shared}{_foreignLanguage=CPreProcessor}

--_tabledef-MyMake=main
--_tabledef-MyMake=cflags

--_mtable-regex-MyMake=main/[A-Z0-9:_]+(FLAGS|DEF)([A-Z0-9:_]+)?[ \t]*[+:]?=[ \t]*//{icase}{tenter=cflags}
--_mtable-regex-MyMake=main/.//

--_mtable-regex-MyMake=cflags/-D([^= \t\n]+)/\1/d/{_language=CPreProcessor}
--_mtable-regex-MyMake=cflags/\\.//
--_mtable-regex-MyMake=cflags/.//
--_mtable-regex-MyMake=cflags/[\n]//{tleave}

an input:

$ cat /tmp/Makefile
REALMODE_CFLAGS	:= -std=gnu11 -m16 -g -Os -DDISABLE_BRANCH_PROFILING -D__DISABLE_EXPORTS \
		   -Wall -Wstrict-prototypes -march=i386 -mregparm=3 \
		   -fno-strict-aliasing -fomit-frame-pointer -fno-pic \
		   -DMYDEF=1 \
		   -mno-mmx -mno-sse $(call cc-option,-fcf-protection=none)

KBUILD_CPPFLAGS := -D__KERNEL__
KBUILD_RUSTFLAGS := $(rust_common_flags) \
		    -Cpanic=abort -Cembed-bitcode=n -Clto=n \
		    -Cforce-unwind-tables=n -Ccodegen-units=1 \
		    -Csymbol-mangling-version=v0 \
		    -Crelocation-model=static \
		    -Zfunction-sections=n \
		    -Dclippy::float_arithmetic

KBUILD_AFLAGS_KERNEL :=
KBUILD_CFLAGS_KERNEL :=
KBUILD_RUSTFLAGS_KERNEL :=
KBUILD_AFLAGS_MODULE  := -DMODULE
KBUILD_CFLAGS_MODULE  := -DMODULE
KBUILD_RUSTFLAGS_MODULE := --cfg MODULE

the result:

$ ./ctags --sort=no --options=/tmp/make-flags.ctags -o - /tmp/Makefile 
REALMODE_CFLAGS	/tmp/Makefile	/^REALMODE_CFLAGS	:= -std=gnu11 -m16 -g -Os -DDISABLE_BRANCH_PROFILING -D__DISABLE_EXPORTS \\$/;"	m
KBUILD_CPPFLAGS	/tmp/Makefile	/^KBUILD_CPPFLAGS := -D__KERNEL__$/;"	m
KBUILD_RUSTFLAGS	/tmp/Makefile	/^KBUILD_RUSTFLAGS := $(rust_common_flags) \\$/;"	m
KBUILD_AFLAGS_KERNEL	/tmp/Makefile	/^KBUILD_AFLAGS_KERNEL :=$/;"	m
KBUILD_CFLAGS_KERNEL	/tmp/Makefile	/^KBUILD_CFLAGS_KERNEL :=$/;"	m
KBUILD_RUSTFLAGS_KERNEL	/tmp/Makefile	/^KBUILD_RUSTFLAGS_KERNEL :=$/;"	m
KBUILD_AFLAGS_MODULE	/tmp/Makefile	/^KBUILD_AFLAGS_MODULE  := -DMODULE$/;"	m
KBUILD_CFLAGS_MODULE	/tmp/Makefile	/^KBUILD_CFLAGS_MODULE  := -DMODULE$/;"	m
KBUILD_RUSTFLAGS_MODULE	/tmp/Makefile	/^KBUILD_RUSTFLAGS_MODULE := --cfg MODULE$/;"	m
DISABLE_BRANCH_PROFILING	/tmp/Makefile	/^REALMODE_CFLAGS	:= -std=gnu11 -m16 -g -Os -DDISABLE_BRANCH_PROFILING -D__DISABLE_EXPORTS \\$/;"	d
__DISABLE_EXPORTS	/tmp/Makefile	/^REALMODE_CFLAGS	:= -std=gnu11 -m16 -g -Os -DDISABLE_BRANCH_PROFILING -D__DISABLE_EXPORTS \\$/;"	d
MYDEF	/tmp/Makefile	/^		   -DMYDEF=1 \\$/;"	d
__KERNEL__	/tmp/Makefile	/^KBUILD_CPPFLAGS := -D__KERNEL__$/;"	d
clippy::float_arithmetic	/tmp/Makefile	/^		    -Dclippy::float_arithmetic$/;"	d
MODULE	/tmp/Makefile	/^KBUILD_AFLAGS_MODULE  := -DMODULE$/;"	d
MODULE	/tmp/Makefile	/^KBUILD_CFLAGS_MODULE  := -DMODULE$/;"	d

This is not specific to Makefiles in the Linux kernel.
I want to implement this in C, not regex.

A parser in ctags can have '--extras-` option.
The option can be used to turn on and off the heuristic.
e.g. ctags --extras-Make=+{extract-D-as-cppmacro} ...

[tleb]

Oh wow that's cool stuff. Once you get closer to submitting patches, don't forget to add comments. It can be read, but it is not straight forward to understand.

I want to implement this in C, not regex.

You mean implement a new program that does tokenization of Makefiles? If yes, can you argument a bit why you think that would be helpful?

I would get the point if it was to switch from regex to some more reliable parsing method. But if the goal is to reimplement a similar behavior, maybe regular expressions passed to ctags can be enough?

[masatake]

You mean implement a new program that does tokenization of Makefiles? If yes, can you argument a bit why you think that would be helpful?

The /tmp/make-flags.ctags, where regular expressions are defined, extends the built-in Make parser in ctags. Though the build-system of ctags has the ability to convert a .ctags file where a new parser is defined, it cannot convert a .ctags file where a built-i parser is extended. Therefore, the feature I demonstrated in /tmp/make-flags.ctags cannot be directly integrated into ctags executable. That means people who want to use the feature must copy and paste make-flags.ctags to one's ~/.ctags.d/something.ctags.

If we rewrite make-flags.ctags in C language as parts of the built-in Make file parser, the copy-and-pate is needed a all. (However, the user must update the ctags executable.)

I would get the point if it was to switch from regex to some more reliable parsing method. But if the goal is to reimplement a similar behavior, maybe regular expressions passed to ctags can be enough?

The rewriting doesn't improve the reliability much, but I would like to pursue making ctags batteries included (https://en.wikipedia.org/wiki/Batteries_Included). Of course, this is my ideal. Users can utilize .ctags as they want, of course.

Anyway, the rewriting is done.

$ cat /tmp/Makefile                                              
REALMODE_CFLAGS	:= -std=gnu11 -m16 -g -Os -DDISABLE_BRANCH_PROFILING -D__DISABLE_EXPORTS \
		   -Wall -Wstrict-prototypes -march=i386 -mregparm=3 \
		   -fno-strict-aliasing -fomit-frame-pointer -fno-pic \
		   -DMYDEF=1 \
		   -mno-mmx -mno-sse $(call cc-option,-fcf-protection=none)

KBUILD_CPPFLAGS := -D__KERNEL__
KBUILD_RUSTFLAGS := $(rust_common_flags) \
		    -Cpanic=abort -Cembed-bitcode=n -Clto=n \
		    -Cforce-unwind-tables=n -Ccodegen-units=1 \
		    -Csymbol-mangling-version=v0 \
		    -Crelocation-model=static \
		    -Zfunction-sections=n \
		    -Dclippy::float_arithmetic

KBUILD_AFLAGS_KERNEL :=
KBUILD_CFLAGS_KERNEL :=
KBUILD_RUSTFLAGS_KERNEL :=
KBUILD_AFLAGS_MODULE  := -DMODULE
KBUILD_CFLAGS_MODULE  := -DMODULE
KBUILD_RUSTFLAGS_MODULE := --cfg MODULE

all:
	$(CC) -DFOO ...
$  ./ctags -o - --sort=no  /tmp/Makefile
REALMODE_CFLAGS	/tmp/Makefile	/^REALMODE_CFLAGS	:= -std=gnu11 -m16 -g -Os -DDISABLE_BRANCH_PROFILING -D__DISABLE_EXPORTS \\$/;"	m
KBUILD_CPPFLAGS	/tmp/Makefile	/^KBUILD_CPPFLAGS := -D__KERNEL__$/;"	m
KBUILD_RUSTFLAGS	/tmp/Makefile	/^KBUILD_RUSTFLAGS := $(rust_common_flags) \\$/;"	m
KBUILD_AFLAGS_KERNEL	/tmp/Makefile	/^KBUILD_AFLAGS_KERNEL :=$/;"	m
KBUILD_CFLAGS_KERNEL	/tmp/Makefile	/^KBUILD_CFLAGS_KERNEL :=$/;"	m
KBUILD_RUSTFLAGS_KERNEL	/tmp/Makefile	/^KBUILD_RUSTFLAGS_KERNEL :=$/;"	m
KBUILD_AFLAGS_MODULE	/tmp/Makefile	/^KBUILD_AFLAGS_MODULE  := -DMODULE$/;"	m
KBUILD_CFLAGS_MODULE	/tmp/Makefile	/^KBUILD_CFLAGS_MODULE  := -DMODULE$/;"	m
KBUILD_RUSTFLAGS_MODULE	/tmp/Makefile	/^KBUILD_RUSTFLAGS_MODULE := --cfg MODULE$/;"	m
all	/tmp/Makefile	/^all:$/;"	t
$  ./ctags -o - --sort=no --extras-Make='+{CppDef}' /tmp/Makefile
REALMODE_CFLAGS	/tmp/Makefile	/^REALMODE_CFLAGS	:= -std=gnu11 -m16 -g -Os -DDISABLE_BRANCH_PROFILING -D__DISABLE_EXPORTS \\$/;"	m
DISABLE_BRANCH_PROFILING	/tmp/Makefile	/^REALMODE_CFLAGS	:= -std=gnu11 -m16 -g -Os -DDISABLE_BRANCH_PROFILING -D__DISABLE_EXPORTS \\$/;"	d
__DISABLE_EXPORTS	/tmp/Makefile	/^REALMODE_CFLAGS	:= -std=gnu11 -m16 -g -Os -DDISABLE_BRANCH_PROFILING -D__DISABLE_EXPORTS \\$/;"	d
MYDEF	/tmp/Makefile	/^		   -DMYDEF=1 \\$/;"	d
KBUILD_CPPFLAGS	/tmp/Makefile	/^KBUILD_CPPFLAGS := -D__KERNEL__$/;"	m
__KERNEL__	/tmp/Makefile	/^KBUILD_CPPFLAGS := -D__KERNEL__$/;"	d
KBUILD_RUSTFLAGS	/tmp/Makefile	/^KBUILD_RUSTFLAGS := $(rust_common_flags) \\$/;"	m
clippy::float_arithmetic	/tmp/Makefile	/^		    -Dclippy::float_arithmetic$/;"	d
KBUILD_AFLAGS_KERNEL	/tmp/Makefile	/^KBUILD_AFLAGS_KERNEL :=$/;"	m
KBUILD_CFLAGS_KERNEL	/tmp/Makefile	/^KBUILD_CFLAGS_KERNEL :=$/;"	m
KBUILD_RUSTFLAGS_KERNEL	/tmp/Makefile	/^KBUILD_RUSTFLAGS_KERNEL :=$/;"	m
KBUILD_AFLAGS_MODULE	/tmp/Makefile	/^KBUILD_AFLAGS_MODULE  := -DMODULE$/;"	m
MODULE	/tmp/Makefile	/^KBUILD_AFLAGS_MODULE  := -DMODULE$/;"	d
KBUILD_CFLAGS_MODULE	/tmp/Makefile	/^KBUILD_CFLAGS_MODULE  := -DMODULE$/;"	m
MODULE	/tmp/Makefile	/^KBUILD_CFLAGS_MODULE  := -DMODULE$/;"	d
KBUILD_RUSTFLAGS_MODULE	/tmp/Makefile	/^KBUILD_RUSTFLAGS_MODULE := --cfg MODULE$/;"	m
all	/tmp/Makefile	/^all:$/;"	t
FOO	/tmp/Makefile	/^	$(CC) -DFOO ...$/;"	d

The option, --extras-Make='+{CppDef}', is for enabling the feature. No .ctags is needed.

[tleb]

Ah, it makes much more sense. Thanks for the explanation, I'm not well versed enough into universal-ctags.

So, to move forward with this:

---

Edge-cases: what are the edge-cases like? Here is what I can think of.

• We'll detect -Dfoo arguments to all programs. That might be really problematic with some tools and in some codebases. It doesn't look like a big issue in the kernel from what I've looked at (grepping for -D in all Makefiles).

• We won't parse -D foo which is supported by GCC.

  This one would give loads of false positives. I don't know why but when people pass macros to their compiler they use -Dfoo but when they pass an argument to any other tool they do -D foo. -D<space> on the kernel would match many arguments passed to install and other various scripts (sphinx for example).

  We do miss some actual $(CC) -D values though, see this Makefile as example.

• We won't parse -D'foo' or -D"foo". Here are the relevant matches on the kernel:

  arch/mips/boot/compressed/Makefile:31:
  ... -D"VMLINUX_LOAD_ADDRESS_ULL=$(VMLINUX_LOAD_ADDRESS)ull"
  
  drivers/net/wireguard/Makefile:1:
  ... -D'pr_fmt(fmt)=KBUILD_MODNAME ": " fmt'
  
  tools/testing/selftests/wireguard/qemu/Makefile:396:
  ... -D'IPUTILS_VERSION(f)=f'
  

• Anything else?

---

Testing: is there any testing we can do?

I could exhaustively read through the list for a big project to just ensure we don't do anything dumb. That does not sound like too much work once the output is deduplicated.

---

Thanks @masatake, it is great to have direct upstream ctags support!