Re: sparse with 4.10?

Re: sparse with 4.10?

[Luc Van Oostenryck]

On Tue, Apr 4, 2017 at 4:14 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Hey,
>
> Compiling modules against 4.10 and checking with the latest sparse git
> master yields these warnings for each and every file:
>
> ./include/linux/mm.h:1713:47: warning: unknown expression (14 0)
> ./include/linux/mm.h:1714:16: warning: call with no type!
>
> That corresponds to this function:
>
> static struct page *pmd_to_page(pmd_t *pmd)
> {
>        unsigned long mask = ~(PTRS_PER_PMD * sizeof(pmd_t) - 1);
>        return virt_to_page((void *)((unsigned long) pmd & mask));
> }
>
> Something broken with the parsing?

+ CC: mailing list

I can't reproduce this on arch/x86, neither with defconfig, nor with
allyesconfig.

Lastest sparse git master is fbbfc7309, right?
Can you send your .config and/or tell a bit more about anything
unusual you could have?

-- Luc Van Oostenryck

Re: sparse with 4.10?

[Jason A. Donenfeld]

[-- Attachment #1: Type: text/plain, Size: 320 bytes --]

Hey guys,

Sorry for the delay. I finally produced an extremely minimal test case
that can reproduce this bug. Please see the attached tarball. Hit
make, and you'll see the error:

./include/linux/mm.h:1766:47: warning: unknown expression (14 0)
./include/linux/mm.h:1767:16: warning: call with no type!

Regards,
Jason

[-- Attachment #2: sparse-bug.tar.xz --]
[-- Type: application/x-xz, Size: 524 bytes --]

Re: sparse with 4.10?

[Luc Van Oostenryck]

On Sat, Sep 16, 2017 at 02:25:18PM +0200, Jason A. Donenfeld wrote:
> Hey guys,
> 
> Sorry for the delay. I finally produced an extremely minimal test case
> that can reproduce this bug. Please see the attached tarball. Hit
> make, and you'll see the error:
> 
> ./include/linux/mm.h:1766:47: warning: unknown expression (14 0)
> ./include/linux/mm.h:1767:16: warning: call with no type!


Thanks, I can now reproduce this.

It's quite strange.

It's related to the fact that you include mm.h via the '-include' flag.
If you include it with '#include' (as it should be), there is no problem.
Same when trying with the result of the preprocessing.
But all three should have the same effect.

The error message itself has somethng to do with the type 'pmd_t'
but again I don't see anything wrong there and I suppose it's because
of the point above.

I'll need to investigate this a bit more.

-- Luc Van Oostenryck

[PATCH 0/2] fix missing evaluation when using '-include <file>'

[Luc Van Oostenryck]

This mini series is to fix a bug where functions included via
'-include <file>' are not evaluated and thus not expanded or
linearized as they should be.

Thanks to Jason A. Donenfeld for catching that and having enough
perseverance to create a small reproducer.

----------------------------------------------------------------

The patches are also available in the git repository at:

  git://github.com/lucvoo/sparse.git fix-include-eval

for you to fetch changes up to 36d1d8b9a453c1f7c215136624198aee9a6f174c:

  fix: missing evaluate with '-include' (2017-09-17 11:54:23 +0200)

----------------------------------------------------------------
Luc Van Oostenryck (2):
      fix: missing evaluate with '-include' : add testcase
      fix: missing evaluate with '-include'

 lib.c                       | 14 ++++++++++++++
 validation/include-eval.c   |  6 ++++++
 validation/include-eval.inc | 12 ++++++++++++
 3 files changed, 32 insertions(+)
 create mode 100644 validation/include-eval.c
 create mode 100644 validation/include-eval.inc

[PATCH 1/2] fix: missing evaluate with '-include' : add testcase

[Luc Van Oostenryck]

Reported-by: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/include-eval.c   |  7 +++++++
 validation/include-eval.inc | 12 ++++++++++++
 2 files changed, 19 insertions(+)
 create mode 100644 validation/include-eval.c
 create mode 100644 validation/include-eval.inc

diff --git a/validation/include-eval.c b/validation/include-eval.c
new file mode 100644
index 000000000..bf9bf63fe
--- /dev/null
+++ b/validation/include-eval.c
@@ -0,0 +1,7 @@
+/* nothing */
+
+/*
+ * check-name: include-eval.c
+ * check-command: sparse -include ./include-eval.inc $file
+ * check-known-to-fail
+ */
diff --git a/validation/include-eval.inc b/validation/include-eval.inc
new file mode 100644
index 000000000..e2561ca41
--- /dev/null
+++ b/validation/include-eval.inc
@@ -0,0 +1,12 @@
+typedef unsigned long long_t;
+
+inline
+static unsigned int ok(void)
+{
+	return sizeof(long_t);
+}
+
+static unsigned int ko(void)
+{
+	return sizeof(long_t);
+}
-- 
2.14.0

[PATCH 2/2] fix: missing evaluate with '-include'

[Luc Van Oostenryck]

Symbols declared or defined in files directly or indirectly
given in the command line and thus processed via sparse()
are always automatically evaluated.
Symbols processed via sparse_initialize(), thus the predefined
ones, are not systematically evaluated, which is normaly fine
as they are just declarations and will be evaluated when needed.

However, if the command line include a file via '-include <file>',
the symbols of this file will also not be evaluated, which is still
fine as long as it's only definitions. But, in the rare and odd cases
where the file should contains a function definition, this function
will not be evaluated and it's expansion and linearization won't happen
as it should have.

Fix this by evaluating the symbols issued from sparse_intialize()
like it is done for sparse().

Reported-by: Jason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 lib.c                     | 14 ++++++++++++++
 validation/include-eval.c |  1 -
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/lib.c b/lib.c
index 4602cf85d..b255fec14 100644
--- a/lib.c
+++ b/lib.c
@@ -1351,6 +1351,20 @@ struct symbol_list *sparse_initialize(int argc, char **argv, struct string_list
 		 */
 		protect_token_alloc();
 	}
+	/*
+	 * Evaluate the complete symbol list
+	 * Note: This is not needed for normal cases.
+	 *	 These symbols should only be predefined defines and
+	 *	 declaratons which will be evaluated later, when needed.
+	 *	 This is also the case when a file is directly included via
+	 *	 '-include <file>' on the command line *AND* the file only
+	 *	 contains defines, declarations and inline definitions.
+	 *	 However, in the rare cases where the given file should
+	 *	 contain some definitions, these will never be evaluated
+	 *	 and thus won't be able to be linearized correctly.
+	 *	 Hence the evaluate_symbol_list() here under.
+	 */
+	evaluate_symbol_list(list);
 	return list;
 }
 
diff --git a/validation/include-eval.c b/validation/include-eval.c
index bf9bf63fe..1a91dab84 100644
--- a/validation/include-eval.c
+++ b/validation/include-eval.c
@@ -3,5 +3,4 @@
 /*
  * check-name: include-eval.c
  * check-command: sparse -include ./include-eval.inc $file
- * check-known-to-fail
  */
-- 
2.14.0

Re: [PATCH 0/2] fix missing evaluation when using '-include <file>'

[Jason A. Donenfeld]

I can confirm this patchfix works well on my slightly more complicated
codebase that was encountering this bug.

By the way, I love the idea of first adding the _failing_ test with
check-known-to-fail, and then removing that flag when the commit that
fixes it lands. Very nice.

Re: [PATCH 0/2] fix missing evaluation when using '-include <file>'

[Luc Van Oostenryck]

On Sun, Sep 17, 2017 at 1:59 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> I can confirm this patchfix works well on my slightly more complicated
> codebase that was encountering this bug.

Good.
Thanks for the extra testing.

> By the way, I love the idea of first adding the _failing_ test with
> check-known-to-fail, and then removing that flag when the commit that
> fixes it lands. Very nice.

Yes, it certainly helps to test and to see what's happening.

-- Luc

Re: [PATCH 0/2] fix missing evaluation when using '-include <file>'

[Luc Van Oostenryck]

On Sun, Sep 17, 2017 at 9:02 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Sun, Sep 17, 2017 at 1:59 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>> I can confirm this patchfix works well on my slightly more complicated
>> codebase that was encountering this bug.
>
> Good.
> Thanks for the extra testing.

This is 7 weeks old now. Given the lack of reaction from the maintainer
I've now pushed this to my dev tree at:
  git://github.com/lucvoo/sparse-dev.git master


-- Luc Van Oostenryck

Re: [PATCH 0/2] fix missing evaluation when using '-include <file>'

[Jason A. Donenfeld]

Any chance of this getting merged into the real sparse tree?

Jason

Re: [PATCH 0/2] fix missing evaluation when using '-include <file>'

[Luc Van Oostenryck]

On Tue, Jan 30, 2018 at 02:23:11PM +0100, Jason A. Donenfeld wrote:
> Any chance of this getting merged into the real sparse tree?
> 
> Jason

Chris,

Can you please take a look at this one, please?

-- Luc

Re: [PATCH 0/2] fix missing evaluation when using '-include <file>'

[Christopher Li]

On Sat, Feb 10, 2018 at 12:02 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Tue, Jan 30, 2018 at 02:23:11PM +0100, Jason A. Donenfeld wrote:
>> Any chance of this getting merged into the real sparse tree?
>>
>> Jason
>
> Chris,
>
> Can you please take a look at this one, please?

Hi, I take a look at this series. It is this series, right?

https://patchwork.kernel.org/patch/9954727/

It looks fine to me. I do want to apply them.

Do you want to make this 2 patch series pullable from master
or I just apply them as patch?

Chris

Re: [PATCH 0/2] fix missing evaluation when using '-include <file>'

[Luc Van Oostenryck]

On Tue, Feb 13, 2018 at 03:49:56PM -0800, Christopher Li wrote:
> On Sat, Feb 10, 2018 at 12:02 PM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > On Tue, Jan 30, 2018 at 02:23:11PM +0100, Jason A. Donenfeld wrote:
> >> Any chance of this getting merged into the real sparse tree?
> >>
> >> Jason
> >
> > Chris,
> >
> > Can you please take a look at this one, please?
> 
> Hi, I take a look at this series. It is this series, right?
> 
> https://patchwork.kernel.org/patch/9954727/

These two patches:
https://patchwork.kernel.org/patch/9954725/
https://patchwork.kernel.org/patch/9954727/

> It looks fine to me. I do want to apply them.
> 
> Do you want to make this 2 patch series pullable from master
> or I just apply them as patch?

Just apply them, please.

-- Luc

Re: [PATCH 0/2] fix missing evaluation when using '-include <file>'

[Jason A. Donenfeld]

Hi Christopher,

Could you apply those two patches, and then afterwards, cut a 0.5.2 release?

Thanks,
Jason

Re: [PATCH 0/2] fix missing evaluation when using '-include <file>'

[Randy Dunlap]

On 02/14/2018 04:15 AM, Jason A. Donenfeld wrote:
> Hi Christopher,
> 
> Could you apply those two patches, and then afterwards, cut a 0.5.2 release?
> 
> Thanks,
> Jason

It would be useful for continuous kernel build bots to have the
"ignore indirect_branch attribute" patch as well.

thanks,
-- 
~Randy