Re: [PATCH] sparse/pre-process: introduce "dissect_mode" option to fix dissect/semind

Hi Oleg,

Slowly catching up my back log from the holidays.

On Wed, Dec 17, 2025 at 7:26 AM Oleg Nesterov <[email protected]> wrote:
>
> I don't quite understand why does expand() -> collect_arg() path
> update ->pos for each token in the input *list, but this breaks
> dissect and thus semind.

That is a good question, I don't understand why it did that either. I
did some digging, inside macro argument list expansion, the "#include
" is not allowed. It is not possible to switch streams here. The
"pos.pos" is for human consumption anyway, it has no effect on the IR
generation. The only visible effect as far as I can tell is related to
the preprocessor "-E" in lib.c:

        if (preprocess_only) {
                while (!eof_token(token)) {
                        int prec = 1;
                        struct token *next = token->next;
                        const char *separator = "";
                        if (next->pos.whitespace)
                                separator = " ";
                        if (next->pos.newline) {
                                separator = "\n\t\t\t\t\t";
                                prec = next->pos.pos; <--- use pos as
indentation level.
                                if (prec > 4)
                                        prec = 4;
                        }
                        printf("%s%.*s", show_token(token), prec, separator);
                        token = next;

The "-E" output has some indentation enhancement to turn space into
tab level indentation. This "pos" assignment tries to align the
indentation context of the input arguments to the same level of the
expanding macro name.

> Test-case:
>
>         $ cat -n PP_POS.c
>              1  #define READ_ONCE(x) x
>              2  #define WRITE_ONCE(x, y) x = y
>              3
>              4  int R, W;
>              5
>              6  void func(void)
>              7  {
>              8    WRITE_ONCE(
>              9       W,

With your patch, when doing "-E", the W will get indentation deeper
than WRITE_ONCE.

>             10       READ_ONCE(R)
>             11    );
>             12  }
>
>         $ ./test-dissect PP_POS.c
>            4:5                    def   v R                                int
>            4:8                    def   v W                                int
>            6:6                    def   f func                             void ( ... )
>            8:3   func             -w-   v W                                int
>            8:3   func             -r-   v R                                int
>
> The reported positions of the usage of R and W are wrong,
> and thus ./semind doesn't work:

It seems to me this enhancement can be used on other macro related
expansions as well.

>
>         $ ./semind add PP_POS.c
>         $ ./semind search -l PP_POS.c:10:16
>
> With this patch:
>
>         $ ./test-dissect PP_POS.c
>            4:5                    def   v R                                int
>            4:8                    def   v W                                int
>            6:6                    def   f func                             void ( ... )
>            9:6   func             -w-   v W                                int
>           10:16  func             -r-   v R                                int
>
>         $ ./semind add PP_POS.c
>         $ ./semind search -l PP_POS.c:10:16
>         (def) PP_POS.c  4       5               int R, W;
>         (-r-) PP_POS.c  10      16      func    READ_ONCE(R)
>
> Signed-off-by: Oleg Nesterov <[email protected]>
> ---
>  dissect.c     | 1 +
>  options.c     | 1 +
>  options.h     | 1 +
>  pre-process.c | 8 +++++---
>  4 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/dissect.c b/dissect.c
> index a6003afa..5fed8e22 100644
> --- a/dissect.c
> +++ b/dissect.c
> @@ -714,6 +714,7 @@ end:
>
>  void dissect(struct reporter *rep, struct string_list *filelist)
>  {
> +       dissect_mode = 1;

I don't think we need dissect_mode. I am leaning towards enabling it
all the time, maybe except for the preprocessor only mode.

>         reporter = rep;
>
>         DO_LIST(filelist, file, do_file(file));
> diff --git a/options.c b/options.c
> index 6ee4d878..0f207e80 100644
> --- a/options.c
> +++ b/options.c
> @@ -71,6 +71,7 @@ int dump_macro_defs = 0;
>  int dump_macros_only = 0;
>
>  int dissect_show_all_symbols = 0;
> +int dissect_mode = 0;
>
>  unsigned long fdump_ir;
>  int fhosted = 1;
> diff --git a/options.h b/options.h
> index c2a9551a..b559254d 100644
> --- a/options.h
> +++ b/options.h
> @@ -71,6 +71,7 @@ extern int dump_macro_defs;
>  extern int dump_macros_only;
>
>  extern int dissect_show_all_symbols;
> +extern int dissect_mode;
>
>  extern unsigned long fdump_ir;
>  extern int fhosted;
> diff --git a/pre-process.c b/pre-process.c
> index 3fb25082..64445881 100644
> --- a/pre-process.c
> +++ b/pre-process.c
> @@ -294,9 +294,11 @@ static struct token *collect_arg(struct token *prev, int vararg, struct position
>                 } else if (match_op(next, ',') && !nesting && !vararg) {
>                         break;
>                 }
> -               next->pos.stream = pos->stream;
> -               next->pos.line = pos->line;
> -               next->pos.pos = pos->pos;
> +               if (!dissect_mode) {
> +                       next->pos.stream = pos->stream;
> +                       next->pos.line = pos->line;
> +                       next->pos.pos = pos->pos;
> +               }

Maybe change it to "if (preprocess_only)", and fix all the validation
error output of the checker. What do you say?

Overall I feel that without this position overwrite is better to
locate the real location of the argument of the macro. If anyone knows
another reason we should do the position overwrite, please let me
know.

Alternatively we can also fix the preprocessor "-E" indentation
output. Might not be worth the complexity.

Chris

>                 next->pos.newline = 0;
>                 p = &next->next;
>         }
> --
> 2.52.0
>
>