All of lore.kernel.org
 help / color / mirror / Atom feed
* [linux-next:master 1430/3917] drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255
@ 2021-03-11 13:06 ` kernel test robot
  0 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2021-03-11 13:06 UTC (permalink / raw)
  To: Maximilian Luz
  Cc: kbuild-all, clang-built-linux, Linux Memory Management List,
	Hans de Goede

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

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head:   98546348153dee5f8ced572fd6c4690461d20f51
commit: 797e78564634275ed4fe6b3f586c4b96eb1d86bc [1430/3917] platform/surface: aggregator_registry: Add base device hub
config: arm64-randconfig-r026-20210311 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 574a9dabc63ba1e7a04c08d4bde2eacd61b44ce1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=797e78564634275ed4fe6b3f586c4b96eb1d86bc
        git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
        git fetch --no-tags linux-next master
        git checkout 797e78564634275ed4fe6b3f586c4b96eb1d86bc
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from drivers/platform/surface/surface_aggregator_registry.c:12:
   In file included from include/linux/acpi.h:35:
   In file included from include/acpi/acpi_io.h:7:
   In file included from arch/arm64/include/asm/acpi.h:12:
   include/linux/efi.h:1093:34: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
           status = get_var(L"SecureBoot", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size,
                                           ^
   include/linux/efi.h:1101:24: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
           get_var(L"SetupMode", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size, &setupmode);
                                 ^
>> drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255 [-Wconstant-conversion]
           { SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
           ~                      ^~~~~~~~~~~~
   include/linux/surface_aggregator/device.h:71:23: note: expanded from macro 'SSAM_ANY_IID'
   #define SSAM_ANY_IID            0xffff
                                   ^~~~~~
   include/linux/surface_aggregator/device.h:126:63: note: expanded from macro 'SSAM_VDEV'
           SSAM_DEVICE(SSAM_DOMAIN_VIRTUAL, SSAM_VIRTUAL_TC_##cat, tid, iid, fun)
                                                                        ^~~
   include/linux/surface_aggregator/device.h:102:41: note: expanded from macro 'SSAM_DEVICE'
           .instance = ((iid) != SSAM_ANY_IID) ? (iid) : 0,                        \
                                                  ^~~
   3 warnings generated.


vim +398 drivers/platform/surface/surface_aggregator_registry.c

   396	
   397	static const struct ssam_device_id ssam_base_hub_match[] = {
 > 398		{ SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
   399		{ },
   400	};
   401	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32383 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread
* [linux-next:master 1430/3917] drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255
@ 2021-03-11 13:06 ` kernel test robot
  0 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2021-03-11 13:06 UTC (permalink / raw)
  To: kbuild-all

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

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head:   98546348153dee5f8ced572fd6c4690461d20f51
commit: 797e78564634275ed4fe6b3f586c4b96eb1d86bc [1430/3917] platform/surface: aggregator_registry: Add base device hub
config: arm64-randconfig-r026-20210311 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 574a9dabc63ba1e7a04c08d4bde2eacd61b44ce1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=797e78564634275ed4fe6b3f586c4b96eb1d86bc
        git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
        git fetch --no-tags linux-next master
        git checkout 797e78564634275ed4fe6b3f586c4b96eb1d86bc
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from drivers/platform/surface/surface_aggregator_registry.c:12:
   In file included from include/linux/acpi.h:35:
   In file included from include/acpi/acpi_io.h:7:
   In file included from arch/arm64/include/asm/acpi.h:12:
   include/linux/efi.h:1093:34: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
           status = get_var(L"SecureBoot", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size,
                                           ^
   include/linux/efi.h:1101:24: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
           get_var(L"SetupMode", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size, &setupmode);
                                 ^
>> drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255 [-Wconstant-conversion]
           { SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
           ~                      ^~~~~~~~~~~~
   include/linux/surface_aggregator/device.h:71:23: note: expanded from macro 'SSAM_ANY_IID'
   #define SSAM_ANY_IID            0xffff
                                   ^~~~~~
   include/linux/surface_aggregator/device.h:126:63: note: expanded from macro 'SSAM_VDEV'
           SSAM_DEVICE(SSAM_DOMAIN_VIRTUAL, SSAM_VIRTUAL_TC_##cat, tid, iid, fun)
                                                                        ^~~
   include/linux/surface_aggregator/device.h:102:41: note: expanded from macro 'SSAM_DEVICE'
           .instance = ((iid) != SSAM_ANY_IID) ? (iid) : 0,                        \
                                                  ^~~
   3 warnings generated.


vim +398 drivers/platform/surface/surface_aggregator_registry.c

   396	
   397	static const struct ssam_device_id ssam_base_hub_match[] = {
 > 398		{ SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
   399		{ },
   400	};
   401	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 32383 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [linux-next:master 1430/3917] drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255
  2021-03-11 13:06 ` kernel test robot
@ 2021-03-11 13:39   ` Maximilian Luz
  -1 siblings, 0 replies; 12+ messages in thread
From: Maximilian Luz @ 2021-03-11 13:39 UTC (permalink / raw)
  To: kernel test robot
  Cc: kbuild-all, clang-built-linux, Linux Memory Management List,
	Hans de Goede

On 3/11/21 2:06 PM, kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> head:   98546348153dee5f8ced572fd6c4690461d20f51
> commit: 797e78564634275ed4fe6b3f586c4b96eb1d86bc [1430/3917] platform/surface: aggregator_registry: Add base device hub
> config: arm64-randconfig-r026-20210311 (attached as .config)
> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 574a9dabc63ba1e7a04c08d4bde2eacd61b44ce1)
> reproduce (this is a W=1 build):
>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>          chmod +x ~/bin/make.cross
>          # install arm64 cross compiling tool for clang build
>          # apt-get install binutils-aarch64-linux-gnu
>          # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=797e78564634275ed4fe6b3f586c4b96eb1d86bc
>          git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
>          git fetch --no-tags linux-next master
>          git checkout 797e78564634275ed4fe6b3f586c4b96eb1d86bc
>          # save the attached .config to linux build tree
>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>):
> 
>     In file included from drivers/platform/surface/surface_aggregator_registry.c:12:
>     In file included from include/linux/acpi.h:35:
>     In file included from include/acpi/acpi_io.h:7:
>     In file included from arch/arm64/include/asm/acpi.h:12:
>     include/linux/efi.h:1093:34: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
>             status = get_var(L"SecureBoot", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size,
>                                             ^
>     include/linux/efi.h:1101:24: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
>             get_var(L"SetupMode", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size, &setupmode);
>                                   ^
> >> drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255 [-Wconstant-conversion]
>             { SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
>             ~                      ^~~~~~~~~~~~

This is a false positive:

>     include/linux/surface_aggregator/device.h:71:23: note: expanded from macro 'SSAM_ANY_IID'
>     #define SSAM_ANY_IID            0xffff
>                                     ^~~~~~

Here, clang complains that the input is SSAM_ANY_ID. That is a special
value which has special considerations below. The SSAM_DEVICE() and
thus SSAM_VDEV() macros are intended to only allow either __u8 or
SSAM_ANY_ID as input in this place.

>     include/linux/surface_aggregator/device.h:126:63: note: expanded from macro 'SSAM_VDEV'
>             SSAM_DEVICE(SSAM_DOMAIN_VIRTUAL, SSAM_VIRTUAL_TC_##cat, tid, iid, fun)
>                                                                          ^~~
>     include/linux/surface_aggregator/device.h:102:41: note: expanded from macro 'SSAM_DEVICE'
>             .instance = ((iid) != SSAM_ANY_IID) ? (iid) : 0,                        \
>                                                    ^~~

Here is the special handling for SSAM_ANY_ID. So there is an implicit
conversion from iid, which may be __u8, to int (i.e. the type of
SSAM_ANY_ID), but there is at no point any implicit conversion of
SSAM_ANY_ID to __u8, as clang alleges.

Is there any way to silence this warning in particular without
suppressing it (e.g. by explicit casting) when users of this macro
_actually_ input any non-u8 and non-SSAM_ANY_ID value as iid,
leading to an _actual_ implicit value-changing cast?

I believe GCC does get this right and only emits a warning if a
non-u8 _and_ non-SSAM_ANY_ID value is input.

Regards,
Max

>     3 warnings generated.
> 
> 
> vim +398 drivers/platform/surface/surface_aggregator_registry.c
> 
>     396	
>     397	static const struct ssam_device_id ssam_base_hub_match[] = {
>   > 398		{ SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
>     399		{ },
>     400	};
>     401	
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [linux-next:master 1430/3917] drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255
@ 2021-03-11 13:39   ` Maximilian Luz
  0 siblings, 0 replies; 12+ messages in thread
From: Maximilian Luz @ 2021-03-11 13:39 UTC (permalink / raw)
  To: kbuild-all

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

On 3/11/21 2:06 PM, kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> head:   98546348153dee5f8ced572fd6c4690461d20f51
> commit: 797e78564634275ed4fe6b3f586c4b96eb1d86bc [1430/3917] platform/surface: aggregator_registry: Add base device hub
> config: arm64-randconfig-r026-20210311 (attached as .config)
> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 574a9dabc63ba1e7a04c08d4bde2eacd61b44ce1)
> reproduce (this is a W=1 build):
>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>          chmod +x ~/bin/make.cross
>          # install arm64 cross compiling tool for clang build
>          # apt-get install binutils-aarch64-linux-gnu
>          # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=797e78564634275ed4fe6b3f586c4b96eb1d86bc
>          git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
>          git fetch --no-tags linux-next master
>          git checkout 797e78564634275ed4fe6b3f586c4b96eb1d86bc
>          # save the attached .config to linux build tree
>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>):
> 
>     In file included from drivers/platform/surface/surface_aggregator_registry.c:12:
>     In file included from include/linux/acpi.h:35:
>     In file included from include/acpi/acpi_io.h:7:
>     In file included from arch/arm64/include/asm/acpi.h:12:
>     include/linux/efi.h:1093:34: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
>             status = get_var(L"SecureBoot", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size,
>                                             ^
>     include/linux/efi.h:1101:24: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
>             get_var(L"SetupMode", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size, &setupmode);
>                                   ^
> >> drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255 [-Wconstant-conversion]
>             { SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
>             ~                      ^~~~~~~~~~~~

This is a false positive:

>     include/linux/surface_aggregator/device.h:71:23: note: expanded from macro 'SSAM_ANY_IID'
>     #define SSAM_ANY_IID            0xffff
>                                     ^~~~~~

Here, clang complains that the input is SSAM_ANY_ID. That is a special
value which has special considerations below. The SSAM_DEVICE() and
thus SSAM_VDEV() macros are intended to only allow either __u8 or
SSAM_ANY_ID as input in this place.

>     include/linux/surface_aggregator/device.h:126:63: note: expanded from macro 'SSAM_VDEV'
>             SSAM_DEVICE(SSAM_DOMAIN_VIRTUAL, SSAM_VIRTUAL_TC_##cat, tid, iid, fun)
>                                                                          ^~~
>     include/linux/surface_aggregator/device.h:102:41: note: expanded from macro 'SSAM_DEVICE'
>             .instance = ((iid) != SSAM_ANY_IID) ? (iid) : 0,                        \
>                                                    ^~~

Here is the special handling for SSAM_ANY_ID. So there is an implicit
conversion from iid, which may be __u8, to int (i.e. the type of
SSAM_ANY_ID), but there is at no point any implicit conversion of
SSAM_ANY_ID to __u8, as clang alleges.

Is there any way to silence this warning in particular without
suppressing it (e.g. by explicit casting) when users of this macro
_actually_ input any non-u8 and non-SSAM_ANY_ID value as iid,
leading to an _actual_ implicit value-changing cast?

I believe GCC does get this right and only emits a warning if a
non-u8 _and_ non-SSAM_ANY_ID value is input.

Regards,
Max

>     3 warnings generated.
> 
> 
> vim +398 drivers/platform/surface/surface_aggregator_registry.c
> 
>     396	
>     397	static const struct ssam_device_id ssam_base_hub_match[] = {
>   > 398		{ SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
>     399		{ },
>     400	};
>     401	
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [linux-next:master 1430/3917] drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255
  2021-03-11 13:39   ` Maximilian Luz
@ 2021-03-11 18:51     ` Nathan Chancellor
  -1 siblings, 0 replies; 12+ messages in thread
From: Nathan Chancellor @ 2021-03-11 18:51 UTC (permalink / raw)
  To: Maximilian Luz
  Cc: kernel test robot, kbuild-all, clang-built-linux,
	Linux Memory Management List, Hans de Goede

On Thu, Mar 11, 2021 at 02:39:03PM +0100, Maximilian Luz wrote:
> On 3/11/21 2:06 PM, kernel test robot wrote:
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> > head:   98546348153dee5f8ced572fd6c4690461d20f51
> > commit: 797e78564634275ed4fe6b3f586c4b96eb1d86bc [1430/3917] platform/surface: aggregator_registry: Add base device hub
> > config: arm64-randconfig-r026-20210311 (attached as .config)
> > compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 574a9dabc63ba1e7a04c08d4bde2eacd61b44ce1)
> > reproduce (this is a W=1 build):
> >          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >          chmod +x ~/bin/make.cross
> >          # install arm64 cross compiling tool for clang build
> >          # apt-get install binutils-aarch64-linux-gnu
> >          # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=797e78564634275ed4fe6b3f586c4b96eb1d86bc
> >          git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> >          git fetch --no-tags linux-next master
> >          git checkout 797e78564634275ed4fe6b3f586c4b96eb1d86bc
> >          # save the attached .config to linux build tree
> >          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
> > 
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> > 
> > All warnings (new ones prefixed by >>):
> > 
> >     In file included from drivers/platform/surface/surface_aggregator_registry.c:12:
> >     In file included from include/linux/acpi.h:35:
> >     In file included from include/acpi/acpi_io.h:7:
> >     In file included from arch/arm64/include/asm/acpi.h:12:
> >     include/linux/efi.h:1093:34: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
> >             status = get_var(L"SecureBoot", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size,
> >                                             ^
> >     include/linux/efi.h:1101:24: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
> >             get_var(L"SetupMode", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size, &setupmode);
> >                                   ^
> > >> drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255 [-Wconstant-conversion]
> >             { SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
> >             ~                      ^~~~~~~~~~~~
> 
> This is a false positive:
> 
> >     include/linux/surface_aggregator/device.h:71:23: note: expanded from macro 'SSAM_ANY_IID'
> >     #define SSAM_ANY_IID            0xffff
> >                                     ^~~~~~
> 
> Here, clang complains that the input is SSAM_ANY_ID. That is a special
> value which has special considerations below. The SSAM_DEVICE() and
> thus SSAM_VDEV() macros are intended to only allow either __u8 or
> SSAM_ANY_ID as input in this place.
> 
> >     include/linux/surface_aggregator/device.h:126:63: note: expanded from macro 'SSAM_VDEV'
> >             SSAM_DEVICE(SSAM_DOMAIN_VIRTUAL, SSAM_VIRTUAL_TC_##cat, tid, iid, fun)
> >                                                                          ^~~
> >     include/linux/surface_aggregator/device.h:102:41: note: expanded from macro 'SSAM_DEVICE'
> >             .instance = ((iid) != SSAM_ANY_IID) ? (iid) : 0,                        \
> >                                                    ^~~
> 
> Here is the special handling for SSAM_ANY_ID. So there is an implicit
> conversion from iid, which may be __u8, to int (i.e. the type of
> SSAM_ANY_ID), but there is at no point any implicit conversion of
> SSAM_ANY_ID to __u8, as clang alleges.

Looks like we are getting bit by https://llvm.org/pr38789 here (also
tracked at https://github.com/ClangBuiltLinux/linux/issues/92).

> Is there any way to silence this warning in particular without
> suppressing it (e.g. by explicit casting) when users of this macro
> _actually_ input any non-u8 and non-SSAM_ANY_ID value as iid,
> leading to an _actual_ implicit value-changing cast?

This has been worked around once before, commit b27aca2e555f ("soc:
rockchip: work around clang warning"). I am not sure of another way to
silence the warning in specific instances like you described aside from
just fixing clang (which is obviously the correct solution) so that we
will get real warnings.

Unfortunately, the patch that is linked in the LLVM PR above does not
appear to fix this instance.

Cheers,
Nathan

> I believe GCC does get this right and only emits a warning if a
> non-u8 _and_ non-SSAM_ANY_ID value is input.
> 
> Regards,
> Max
> 
> >     3 warnings generated.
> > 
> > 
> > vim +398 drivers/platform/surface/surface_aggregator_registry.c
> > 
> >     396	
> >     397	static const struct ssam_device_id ssam_base_hub_match[] = {
> >   > 398		{ SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
> >     399		{ },
> >     400	};
> >     401	
> > 
> > ---
> > 0-DAY CI Kernel Test Service, Intel Corporation
> > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> > 
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [linux-next:master 1430/3917] drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255
@ 2021-03-11 18:51     ` Nathan Chancellor
  0 siblings, 0 replies; 12+ messages in thread
From: Nathan Chancellor @ 2021-03-11 18:51 UTC (permalink / raw)
  To: kbuild-all

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

On Thu, Mar 11, 2021 at 02:39:03PM +0100, Maximilian Luz wrote:
> On 3/11/21 2:06 PM, kernel test robot wrote:
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> > head:   98546348153dee5f8ced572fd6c4690461d20f51
> > commit: 797e78564634275ed4fe6b3f586c4b96eb1d86bc [1430/3917] platform/surface: aggregator_registry: Add base device hub
> > config: arm64-randconfig-r026-20210311 (attached as .config)
> > compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 574a9dabc63ba1e7a04c08d4bde2eacd61b44ce1)
> > reproduce (this is a W=1 build):
> >          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >          chmod +x ~/bin/make.cross
> >          # install arm64 cross compiling tool for clang build
> >          # apt-get install binutils-aarch64-linux-gnu
> >          # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=797e78564634275ed4fe6b3f586c4b96eb1d86bc
> >          git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> >          git fetch --no-tags linux-next master
> >          git checkout 797e78564634275ed4fe6b3f586c4b96eb1d86bc
> >          # save the attached .config to linux build tree
> >          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
> > 
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> > 
> > All warnings (new ones prefixed by >>):
> > 
> >     In file included from drivers/platform/surface/surface_aggregator_registry.c:12:
> >     In file included from include/linux/acpi.h:35:
> >     In file included from include/acpi/acpi_io.h:7:
> >     In file included from arch/arm64/include/asm/acpi.h:12:
> >     include/linux/efi.h:1093:34: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
> >             status = get_var(L"SecureBoot", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size,
> >                                             ^
> >     include/linux/efi.h:1101:24: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
> >             get_var(L"SetupMode", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size, &setupmode);
> >                                   ^
> > >> drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255 [-Wconstant-conversion]
> >             { SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
> >             ~                      ^~~~~~~~~~~~
> 
> This is a false positive:
> 
> >     include/linux/surface_aggregator/device.h:71:23: note: expanded from macro 'SSAM_ANY_IID'
> >     #define SSAM_ANY_IID            0xffff
> >                                     ^~~~~~
> 
> Here, clang complains that the input is SSAM_ANY_ID. That is a special
> value which has special considerations below. The SSAM_DEVICE() and
> thus SSAM_VDEV() macros are intended to only allow either __u8 or
> SSAM_ANY_ID as input in this place.
> 
> >     include/linux/surface_aggregator/device.h:126:63: note: expanded from macro 'SSAM_VDEV'
> >             SSAM_DEVICE(SSAM_DOMAIN_VIRTUAL, SSAM_VIRTUAL_TC_##cat, tid, iid, fun)
> >                                                                          ^~~
> >     include/linux/surface_aggregator/device.h:102:41: note: expanded from macro 'SSAM_DEVICE'
> >             .instance = ((iid) != SSAM_ANY_IID) ? (iid) : 0,                        \
> >                                                    ^~~
> 
> Here is the special handling for SSAM_ANY_ID. So there is an implicit
> conversion from iid, which may be __u8, to int (i.e. the type of
> SSAM_ANY_ID), but there is at no point any implicit conversion of
> SSAM_ANY_ID to __u8, as clang alleges.

Looks like we are getting bit by https://llvm.org/pr38789 here (also
tracked at https://github.com/ClangBuiltLinux/linux/issues/92).

> Is there any way to silence this warning in particular without
> suppressing it (e.g. by explicit casting) when users of this macro
> _actually_ input any non-u8 and non-SSAM_ANY_ID value as iid,
> leading to an _actual_ implicit value-changing cast?

This has been worked around once before, commit b27aca2e555f ("soc:
rockchip: work around clang warning"). I am not sure of another way to
silence the warning in specific instances like you described aside from
just fixing clang (which is obviously the correct solution) so that we
will get real warnings.

Unfortunately, the patch that is linked in the LLVM PR above does not
appear to fix this instance.

Cheers,
Nathan

> I believe GCC does get this right and only emits a warning if a
> non-u8 _and_ non-SSAM_ANY_ID value is input.
> 
> Regards,
> Max
> 
> >     3 warnings generated.
> > 
> > 
> > vim +398 drivers/platform/surface/surface_aggregator_registry.c
> > 
> >     396	
> >     397	static const struct ssam_device_id ssam_base_hub_match[] = {
> >   > 398		{ SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
> >     399		{ },
> >     400	};
> >     401	
> > 
> > ---
> > 0-DAY CI Kernel Test Service, Intel Corporation
> > https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
> > 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [linux-next:master 1430/3917] drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255
  2021-03-11 18:51     ` Nathan Chancellor
@ 2021-03-12  0:13       ` Maximilian Luz
  -1 siblings, 0 replies; 12+ messages in thread
From: Maximilian Luz @ 2021-03-12  0:13 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: kernel test robot, kbuild-all, clang-built-linux,
	Linux Memory Management List, Hans de Goede

On 3/11/21 7:51 PM, Nathan Chancellor wrote:
> On Thu, Mar 11, 2021 at 02:39:03PM +0100, Maximilian Luz wrote:
>> On 3/11/21 2:06 PM, kernel test robot wrote:
>>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
>>> head:   98546348153dee5f8ced572fd6c4690461d20f51
>>> commit: 797e78564634275ed4fe6b3f586c4b96eb1d86bc [1430/3917] platform/surface: aggregator_registry: Add base device hub
>>> config: arm64-randconfig-r026-20210311 (attached as .config)
>>> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 574a9dabc63ba1e7a04c08d4bde2eacd61b44ce1)
>>> reproduce (this is a W=1 build):
>>>           wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>>           chmod +x ~/bin/make.cross
>>>           # install arm64 cross compiling tool for clang build
>>>           # apt-get install binutils-aarch64-linux-gnu
>>>           # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=797e78564634275ed4fe6b3f586c4b96eb1d86bc
>>>           git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
>>>           git fetch --no-tags linux-next master
>>>           git checkout 797e78564634275ed4fe6b3f586c4b96eb1d86bc
>>>           # save the attached .config to linux build tree
>>>           COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
>>>
>>> If you fix the issue, kindly add following tag as appropriate
>>> Reported-by: kernel test robot <lkp@intel.com>
>>>
>>> All warnings (new ones prefixed by >>):
>>>
>>>      In file included from drivers/platform/surface/surface_aggregator_registry.c:12:
>>>      In file included from include/linux/acpi.h:35:
>>>      In file included from include/acpi/acpi_io.h:7:
>>>      In file included from arch/arm64/include/asm/acpi.h:12:
>>>      include/linux/efi.h:1093:34: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
>>>              status = get_var(L"SecureBoot", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size,
>>>                                              ^
>>>      include/linux/efi.h:1101:24: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
>>>              get_var(L"SetupMode", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size, &setupmode);
>>>                                    ^
>>>>> drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255 [-Wconstant-conversion]
>>>              { SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
>>>              ~                      ^~~~~~~~~~~~
>>
>> This is a false positive:
>>
>>>      include/linux/surface_aggregator/device.h:71:23: note: expanded from macro 'SSAM_ANY_IID'
>>>      #define SSAM_ANY_IID            0xffff
>>>                                      ^~~~~~
>>
>> Here, clang complains that the input is SSAM_ANY_ID. That is a special
>> value which has special considerations below. The SSAM_DEVICE() and
>> thus SSAM_VDEV() macros are intended to only allow either __u8 or
>> SSAM_ANY_ID as input in this place.
>>
>>>      include/linux/surface_aggregator/device.h:126:63: note: expanded from macro 'SSAM_VDEV'
>>>              SSAM_DEVICE(SSAM_DOMAIN_VIRTUAL, SSAM_VIRTUAL_TC_##cat, tid, iid, fun)
>>>                                                                           ^~~
>>>      include/linux/surface_aggregator/device.h:102:41: note: expanded from macro 'SSAM_DEVICE'
>>>              .instance = ((iid) != SSAM_ANY_IID) ? (iid) : 0,                        \
>>>                                                     ^~~
>>
>> Here is the special handling for SSAM_ANY_ID. So there is an implicit
>> conversion from iid, which may be __u8, to int (i.e. the type of
>> SSAM_ANY_ID), but there is at no point any implicit conversion of
>> SSAM_ANY_ID to __u8, as clang alleges.
> 
> Looks like we are getting bit by https://llvm.org/pr38789 here (also
> tracked at https://github.com/ClangBuiltLinux/linux/issues/92).

Thanks for the links! Looks like this is the same issue.

>> Is there any way to silence this warning in particular without
>> suppressing it (e.g. by explicit casting) when users of this macro
>> _actually_ input any non-u8 and non-SSAM_ANY_ID value as iid,
>> leading to an _actual_ implicit value-changing cast?
> 
> This has been worked around once before, commit b27aca2e555f ("soc:
> rockchip: work around clang warning"). I am not sure of another way to
> silence the warning in specific instances like you described aside from
> just fixing clang (which is obviously the correct solution) so that we
> will get real warnings.
> 
> Unfortunately, the patch that is linked in the LLVM PR above does not
> appear to fix this instance.

So what's the way forward here? Ignore this warning for now and wait for
the clang team to fix it or find a workaround?

The best workaround I can come up with is setting the match_flags
parameter directly as macro argument, which would be a bit more verbose
and feel slightly less clean, but would do the trick.

Also note that any changes to those macros would have to be synchronized
with a couple of patches which are still out to non-pdx86 trees, so if
we're working around this it's best to do this as soon as possible.
Specifically talking to Hans here, as those patches all have a
dependency on stuff that's only in the pdx86 tree right now. I.e.
getting the workaround in before setting up a tag for those dependencies
would simplify things.

Regards,
Max

> Cheers,
> Nathan
> 
>> I believe GCC does get this right and only emits a warning if a
>> non-u8 _and_ non-SSAM_ANY_ID value is input.
>>
>> Regards,
>> Max
>>
>>>      3 warnings generated.
>>>
>>>
>>> vim +398 drivers/platform/surface/surface_aggregator_registry.c
>>>
>>>      396	
>>>      397	static const struct ssam_device_id ssam_base_hub_match[] = {
>>>    > 398		{ SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
>>>      399		{ },
>>>      400	};
>>>      401	
>>>
>>> ---
>>> 0-DAY CI Kernel Test Service, Intel Corporation
>>> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>>>
>>


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [linux-next:master 1430/3917] drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255
@ 2021-03-12  0:13       ` Maximilian Luz
  0 siblings, 0 replies; 12+ messages in thread
From: Maximilian Luz @ 2021-03-12  0:13 UTC (permalink / raw)
  To: kbuild-all

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

On 3/11/21 7:51 PM, Nathan Chancellor wrote:
> On Thu, Mar 11, 2021 at 02:39:03PM +0100, Maximilian Luz wrote:
>> On 3/11/21 2:06 PM, kernel test robot wrote:
>>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
>>> head:   98546348153dee5f8ced572fd6c4690461d20f51
>>> commit: 797e78564634275ed4fe6b3f586c4b96eb1d86bc [1430/3917] platform/surface: aggregator_registry: Add base device hub
>>> config: arm64-randconfig-r026-20210311 (attached as .config)
>>> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 574a9dabc63ba1e7a04c08d4bde2eacd61b44ce1)
>>> reproduce (this is a W=1 build):
>>>           wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>>           chmod +x ~/bin/make.cross
>>>           # install arm64 cross compiling tool for clang build
>>>           # apt-get install binutils-aarch64-linux-gnu
>>>           # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=797e78564634275ed4fe6b3f586c4b96eb1d86bc
>>>           git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
>>>           git fetch --no-tags linux-next master
>>>           git checkout 797e78564634275ed4fe6b3f586c4b96eb1d86bc
>>>           # save the attached .config to linux build tree
>>>           COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
>>>
>>> If you fix the issue, kindly add following tag as appropriate
>>> Reported-by: kernel test robot <lkp@intel.com>
>>>
>>> All warnings (new ones prefixed by >>):
>>>
>>>      In file included from drivers/platform/surface/surface_aggregator_registry.c:12:
>>>      In file included from include/linux/acpi.h:35:
>>>      In file included from include/acpi/acpi_io.h:7:
>>>      In file included from arch/arm64/include/asm/acpi.h:12:
>>>      include/linux/efi.h:1093:34: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
>>>              status = get_var(L"SecureBoot", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size,
>>>                                              ^
>>>      include/linux/efi.h:1101:24: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
>>>              get_var(L"SetupMode", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size, &setupmode);
>>>                                    ^
>>>>> drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255 [-Wconstant-conversion]
>>>              { SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
>>>              ~                      ^~~~~~~~~~~~
>>
>> This is a false positive:
>>
>>>      include/linux/surface_aggregator/device.h:71:23: note: expanded from macro 'SSAM_ANY_IID'
>>>      #define SSAM_ANY_IID            0xffff
>>>                                      ^~~~~~
>>
>> Here, clang complains that the input is SSAM_ANY_ID. That is a special
>> value which has special considerations below. The SSAM_DEVICE() and
>> thus SSAM_VDEV() macros are intended to only allow either __u8 or
>> SSAM_ANY_ID as input in this place.
>>
>>>      include/linux/surface_aggregator/device.h:126:63: note: expanded from macro 'SSAM_VDEV'
>>>              SSAM_DEVICE(SSAM_DOMAIN_VIRTUAL, SSAM_VIRTUAL_TC_##cat, tid, iid, fun)
>>>                                                                           ^~~
>>>      include/linux/surface_aggregator/device.h:102:41: note: expanded from macro 'SSAM_DEVICE'
>>>              .instance = ((iid) != SSAM_ANY_IID) ? (iid) : 0,                        \
>>>                                                     ^~~
>>
>> Here is the special handling for SSAM_ANY_ID. So there is an implicit
>> conversion from iid, which may be __u8, to int (i.e. the type of
>> SSAM_ANY_ID), but there is at no point any implicit conversion of
>> SSAM_ANY_ID to __u8, as clang alleges.
> 
> Looks like we are getting bit by https://llvm.org/pr38789 here (also
> tracked at https://github.com/ClangBuiltLinux/linux/issues/92).

Thanks for the links! Looks like this is the same issue.

>> Is there any way to silence this warning in particular without
>> suppressing it (e.g. by explicit casting) when users of this macro
>> _actually_ input any non-u8 and non-SSAM_ANY_ID value as iid,
>> leading to an _actual_ implicit value-changing cast?
> 
> This has been worked around once before, commit b27aca2e555f ("soc:
> rockchip: work around clang warning"). I am not sure of another way to
> silence the warning in specific instances like you described aside from
> just fixing clang (which is obviously the correct solution) so that we
> will get real warnings.
> 
> Unfortunately, the patch that is linked in the LLVM PR above does not
> appear to fix this instance.

So what's the way forward here? Ignore this warning for now and wait for
the clang team to fix it or find a workaround?

The best workaround I can come up with is setting the match_flags
parameter directly as macro argument, which would be a bit more verbose
and feel slightly less clean, but would do the trick.

Also note that any changes to those macros would have to be synchronized
with a couple of patches which are still out to non-pdx86 trees, so if
we're working around this it's best to do this as soon as possible.
Specifically talking to Hans here, as those patches all have a
dependency on stuff that's only in the pdx86 tree right now. I.e.
getting the workaround in before setting up a tag for those dependencies
would simplify things.

Regards,
Max

> Cheers,
> Nathan
> 
>> I believe GCC does get this right and only emits a warning if a
>> non-u8 _and_ non-SSAM_ANY_ID value is input.
>>
>> Regards,
>> Max
>>
>>>      3 warnings generated.
>>>
>>>
>>> vim +398 drivers/platform/surface/surface_aggregator_registry.c
>>>
>>>      396	
>>>      397	static const struct ssam_device_id ssam_base_hub_match[] = {
>>>    > 398		{ SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
>>>      399		{ },
>>>      400	};
>>>      401	
>>>
>>> ---
>>> 0-DAY CI Kernel Test Service, Intel Corporation
>>> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
>>>
>>

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [linux-next:master 1430/3917] drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255
  2021-03-12  0:13       ` Maximilian Luz
@ 2021-03-12  0:18         ` Nick Desaulniers
  -1 siblings, 0 replies; 12+ messages in thread
From: Nick Desaulniers @ 2021-03-12  0:18 UTC (permalink / raw)
  To: Maximilian Luz
  Cc: Nathan Chancellor, kernel test robot, kbuild-all,
	clang-built-linux, Linux Memory Management List, Hans de Goede

On Thu, Mar 11, 2021 at 4:13 PM Maximilian Luz <luzmaximilian@gmail.com> wrote:
>
> On 3/11/21 7:51 PM, Nathan Chancellor wrote:
> > On Thu, Mar 11, 2021 at 02:39:03PM +0100, Maximilian Luz wrote:
> >> On 3/11/21 2:06 PM, kernel test robot wrote:
> >>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> >>> head:   98546348153dee5f8ced572fd6c4690461d20f51
> >>> commit: 797e78564634275ed4fe6b3f586c4b96eb1d86bc [1430/3917] platform/surface: aggregator_registry: Add base device hub
> >>> config: arm64-randconfig-r026-20210311 (attached as .config)
> >>> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 574a9dabc63ba1e7a04c08d4bde2eacd61b44ce1)
> >>> reproduce (this is a W=1 build):
> >>>           wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >>>           chmod +x ~/bin/make.cross
> >>>           # install arm64 cross compiling tool for clang build
> >>>           # apt-get install binutils-aarch64-linux-gnu
> >>>           # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=797e78564634275ed4fe6b3f586c4b96eb1d86bc
> >>>           git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> >>>           git fetch --no-tags linux-next master
> >>>           git checkout 797e78564634275ed4fe6b3f586c4b96eb1d86bc
> >>>           # save the attached .config to linux build tree
> >>>           COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
> >>>
> >>> If you fix the issue, kindly add following tag as appropriate
> >>> Reported-by: kernel test robot <lkp@intel.com>
> >>>
> >>> All warnings (new ones prefixed by >>):
> >>>
> >>>      In file included from drivers/platform/surface/surface_aggregator_registry.c:12:
> >>>      In file included from include/linux/acpi.h:35:
> >>>      In file included from include/acpi/acpi_io.h:7:
> >>>      In file included from arch/arm64/include/asm/acpi.h:12:
> >>>      include/linux/efi.h:1093:34: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
> >>>              status = get_var(L"SecureBoot", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size,
> >>>                                              ^
> >>>      include/linux/efi.h:1101:24: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
> >>>              get_var(L"SetupMode", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size, &setupmode);
> >>>                                    ^
> >>>>> drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255 [-Wconstant-conversion]
> >>>              { SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
> >>>              ~                      ^~~~~~~~~~~~
> >>
> >> This is a false positive:
> >>
> >>>      include/linux/surface_aggregator/device.h:71:23: note: expanded from macro 'SSAM_ANY_IID'
> >>>      #define SSAM_ANY_IID            0xffff
> >>>                                      ^~~~~~
> >>
> >> Here, clang complains that the input is SSAM_ANY_ID. That is a special
> >> value which has special considerations below. The SSAM_DEVICE() and
> >> thus SSAM_VDEV() macros are intended to only allow either __u8 or
> >> SSAM_ANY_ID as input in this place.
> >>
> >>>      include/linux/surface_aggregator/device.h:126:63: note: expanded from macro 'SSAM_VDEV'
> >>>              SSAM_DEVICE(SSAM_DOMAIN_VIRTUAL, SSAM_VIRTUAL_TC_##cat, tid, iid, fun)
> >>>                                                                           ^~~
> >>>      include/linux/surface_aggregator/device.h:102:41: note: expanded from macro 'SSAM_DEVICE'
> >>>              .instance = ((iid) != SSAM_ANY_IID) ? (iid) : 0,                        \
> >>>                                                     ^~~
> >>
> >> Here is the special handling for SSAM_ANY_ID. So there is an implicit
> >> conversion from iid, which may be __u8, to int (i.e. the type of
> >> SSAM_ANY_ID), but there is at no point any implicit conversion of
> >> SSAM_ANY_ID to __u8, as clang alleges.
> >
> > Looks like we are getting bit by https://llvm.org/pr38789 here (also
> > tracked at https://github.com/ClangBuiltLinux/linux/issues/92).
>
> Thanks for the links! Looks like this is the same issue.
>
> >> Is there any way to silence this warning in particular without
> >> suppressing it (e.g. by explicit casting) when users of this macro
> >> _actually_ input any non-u8 and non-SSAM_ANY_ID value as iid,
> >> leading to an _actual_ implicit value-changing cast?
> >
> > This has been worked around once before, commit b27aca2e555f ("soc:
> > rockchip: work around clang warning"). I am not sure of another way to
> > silence the warning in specific instances like you described aside from
> > just fixing clang (which is obviously the correct solution) so that we
> > will get real warnings.
> >
> > Unfortunately, the patch that is linked in the LLVM PR above does not
> > appear to fix this instance.
>
> So what's the way forward here? Ignore this warning for now and wait for
> the clang team to fix it or find a workaround?

I think we should fix this on the Clang side; having your additional
test case will help us refine the initial approach.  Not the highest
priority for us as this was found via randconfig; if it was something
that showed up for defconfigs for every arch that would help us
reprioritize but at the moment we have a lot of higher priority work.
Either way, I appreciate the effort looking into this 0day bot
warning!

>
> The best workaround I can come up with is setting the match_flags
> parameter directly as macro argument, which would be a bit more verbose
> and feel slightly less clean, but would do the trick.
>
> Also note that any changes to those macros would have to be synchronized
> with a couple of patches which are still out to non-pdx86 trees, so if
> we're working around this it's best to do this as soon as possible.
> Specifically talking to Hans here, as those patches all have a
> dependency on stuff that's only in the pdx86 tree right now. I.e.
> getting the workaround in before setting up a tag for those dependencies
> would simplify things.
>
> Regards,
> Max
>
> > Cheers,
> > Nathan
> >
> >> I believe GCC does get this right and only emits a warning if a
> >> non-u8 _and_ non-SSAM_ANY_ID value is input.
> >>
> >> Regards,
> >> Max
> >>
> >>>      3 warnings generated.
> >>>
> >>>
> >>> vim +398 drivers/platform/surface/surface_aggregator_registry.c
> >>>
> >>>      396
> >>>      397    static const struct ssam_device_id ssam_base_hub_match[] = {
> >>>    > 398            { SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
> >>>      399            { },
> >>>      400    };
> >>>      401
> >>>
> >>> ---
> >>> 0-DAY CI Kernel Test Service, Intel Corporation
> >>> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> >>>
> >>
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/7db15c91-db37-69a4-84b7-6f58cca2ee1b%40gmail.com.



-- 
Thanks,
~Nick Desaulniers


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [linux-next:master 1430/3917] drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255
@ 2021-03-12  0:18         ` Nick Desaulniers
  0 siblings, 0 replies; 12+ messages in thread
From: Nick Desaulniers @ 2021-03-12  0:18 UTC (permalink / raw)
  To: kbuild-all

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

On Thu, Mar 11, 2021 at 4:13 PM Maximilian Luz <luzmaximilian@gmail.com> wrote:
>
> On 3/11/21 7:51 PM, Nathan Chancellor wrote:
> > On Thu, Mar 11, 2021 at 02:39:03PM +0100, Maximilian Luz wrote:
> >> On 3/11/21 2:06 PM, kernel test robot wrote:
> >>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> >>> head:   98546348153dee5f8ced572fd6c4690461d20f51
> >>> commit: 797e78564634275ed4fe6b3f586c4b96eb1d86bc [1430/3917] platform/surface: aggregator_registry: Add base device hub
> >>> config: arm64-randconfig-r026-20210311 (attached as .config)
> >>> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 574a9dabc63ba1e7a04c08d4bde2eacd61b44ce1)
> >>> reproduce (this is a W=1 build):
> >>>           wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >>>           chmod +x ~/bin/make.cross
> >>>           # install arm64 cross compiling tool for clang build
> >>>           # apt-get install binutils-aarch64-linux-gnu
> >>>           # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=797e78564634275ed4fe6b3f586c4b96eb1d86bc
> >>>           git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> >>>           git fetch --no-tags linux-next master
> >>>           git checkout 797e78564634275ed4fe6b3f586c4b96eb1d86bc
> >>>           # save the attached .config to linux build tree
> >>>           COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
> >>>
> >>> If you fix the issue, kindly add following tag as appropriate
> >>> Reported-by: kernel test robot <lkp@intel.com>
> >>>
> >>> All warnings (new ones prefixed by >>):
> >>>
> >>>      In file included from drivers/platform/surface/surface_aggregator_registry.c:12:
> >>>      In file included from include/linux/acpi.h:35:
> >>>      In file included from include/acpi/acpi_io.h:7:
> >>>      In file included from arch/arm64/include/asm/acpi.h:12:
> >>>      include/linux/efi.h:1093:34: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
> >>>              status = get_var(L"SecureBoot", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size,
> >>>                                              ^
> >>>      include/linux/efi.h:1101:24: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
> >>>              get_var(L"SetupMode", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size, &setupmode);
> >>>                                    ^
> >>>>> drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255 [-Wconstant-conversion]
> >>>              { SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
> >>>              ~                      ^~~~~~~~~~~~
> >>
> >> This is a false positive:
> >>
> >>>      include/linux/surface_aggregator/device.h:71:23: note: expanded from macro 'SSAM_ANY_IID'
> >>>      #define SSAM_ANY_IID            0xffff
> >>>                                      ^~~~~~
> >>
> >> Here, clang complains that the input is SSAM_ANY_ID. That is a special
> >> value which has special considerations below. The SSAM_DEVICE() and
> >> thus SSAM_VDEV() macros are intended to only allow either __u8 or
> >> SSAM_ANY_ID as input in this place.
> >>
> >>>      include/linux/surface_aggregator/device.h:126:63: note: expanded from macro 'SSAM_VDEV'
> >>>              SSAM_DEVICE(SSAM_DOMAIN_VIRTUAL, SSAM_VIRTUAL_TC_##cat, tid, iid, fun)
> >>>                                                                           ^~~
> >>>      include/linux/surface_aggregator/device.h:102:41: note: expanded from macro 'SSAM_DEVICE'
> >>>              .instance = ((iid) != SSAM_ANY_IID) ? (iid) : 0,                        \
> >>>                                                     ^~~
> >>
> >> Here is the special handling for SSAM_ANY_ID. So there is an implicit
> >> conversion from iid, which may be __u8, to int (i.e. the type of
> >> SSAM_ANY_ID), but there is at no point any implicit conversion of
> >> SSAM_ANY_ID to __u8, as clang alleges.
> >
> > Looks like we are getting bit by https://llvm.org/pr38789 here (also
> > tracked at https://github.com/ClangBuiltLinux/linux/issues/92).
>
> Thanks for the links! Looks like this is the same issue.
>
> >> Is there any way to silence this warning in particular without
> >> suppressing it (e.g. by explicit casting) when users of this macro
> >> _actually_ input any non-u8 and non-SSAM_ANY_ID value as iid,
> >> leading to an _actual_ implicit value-changing cast?
> >
> > This has been worked around once before, commit b27aca2e555f ("soc:
> > rockchip: work around clang warning"). I am not sure of another way to
> > silence the warning in specific instances like you described aside from
> > just fixing clang (which is obviously the correct solution) so that we
> > will get real warnings.
> >
> > Unfortunately, the patch that is linked in the LLVM PR above does not
> > appear to fix this instance.
>
> So what's the way forward here? Ignore this warning for now and wait for
> the clang team to fix it or find a workaround?

I think we should fix this on the Clang side; having your additional
test case will help us refine the initial approach.  Not the highest
priority for us as this was found via randconfig; if it was something
that showed up for defconfigs for every arch that would help us
reprioritize but at the moment we have a lot of higher priority work.
Either way, I appreciate the effort looking into this 0day bot
warning!

>
> The best workaround I can come up with is setting the match_flags
> parameter directly as macro argument, which would be a bit more verbose
> and feel slightly less clean, but would do the trick.
>
> Also note that any changes to those macros would have to be synchronized
> with a couple of patches which are still out to non-pdx86 trees, so if
> we're working around this it's best to do this as soon as possible.
> Specifically talking to Hans here, as those patches all have a
> dependency on stuff that's only in the pdx86 tree right now. I.e.
> getting the workaround in before setting up a tag for those dependencies
> would simplify things.
>
> Regards,
> Max
>
> > Cheers,
> > Nathan
> >
> >> I believe GCC does get this right and only emits a warning if a
> >> non-u8 _and_ non-SSAM_ANY_ID value is input.
> >>
> >> Regards,
> >> Max
> >>
> >>>      3 warnings generated.
> >>>
> >>>
> >>> vim +398 drivers/platform/surface/surface_aggregator_registry.c
> >>>
> >>>      396
> >>>      397    static const struct ssam_device_id ssam_base_hub_match[] = {
> >>>    > 398            { SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
> >>>      399            { },
> >>>      400    };
> >>>      401
> >>>
> >>> ---
> >>> 0-DAY CI Kernel Test Service, Intel Corporation
> >>> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
> >>>
> >>
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe(a)googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/7db15c91-db37-69a4-84b7-6f58cca2ee1b%40gmail.com.



-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [linux-next:master 1430/3917] drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255
  2021-03-12  0:18         ` Nick Desaulniers
@ 2021-03-12  3:39           ` Nathan Chancellor
  -1 siblings, 0 replies; 12+ messages in thread
From: Nathan Chancellor @ 2021-03-12  3:39 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Maximilian Luz, kernel test robot, kbuild-all, clang-built-linux,
	Linux Memory Management List, Hans de Goede

On Thu, Mar 11, 2021 at 04:18:59PM -0800, Nick Desaulniers wrote:
> On Thu, Mar 11, 2021 at 4:13 PM Maximilian Luz <luzmaximilian@gmail.com> wrote:
> >
> > On 3/11/21 7:51 PM, Nathan Chancellor wrote:
> > > On Thu, Mar 11, 2021 at 02:39:03PM +0100, Maximilian Luz wrote:
> > >> On 3/11/21 2:06 PM, kernel test robot wrote:
> > >>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> > >>> head:   98546348153dee5f8ced572fd6c4690461d20f51
> > >>> commit: 797e78564634275ed4fe6b3f586c4b96eb1d86bc [1430/3917] platform/surface: aggregator_registry: Add base device hub
> > >>> config: arm64-randconfig-r026-20210311 (attached as .config)
> > >>> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 574a9dabc63ba1e7a04c08d4bde2eacd61b44ce1)
> > >>> reproduce (this is a W=1 build):
> > >>>           wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > >>>           chmod +x ~/bin/make.cross
> > >>>           # install arm64 cross compiling tool for clang build
> > >>>           # apt-get install binutils-aarch64-linux-gnu
> > >>>           # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=797e78564634275ed4fe6b3f586c4b96eb1d86bc
> > >>>           git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> > >>>           git fetch --no-tags linux-next master
> > >>>           git checkout 797e78564634275ed4fe6b3f586c4b96eb1d86bc
> > >>>           # save the attached .config to linux build tree
> > >>>           COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
> > >>>
> > >>> If you fix the issue, kindly add following tag as appropriate
> > >>> Reported-by: kernel test robot <lkp@intel.com>
> > >>>
> > >>> All warnings (new ones prefixed by >>):
> > >>>
> > >>>      In file included from drivers/platform/surface/surface_aggregator_registry.c:12:
> > >>>      In file included from include/linux/acpi.h:35:
> > >>>      In file included from include/acpi/acpi_io.h:7:
> > >>>      In file included from arch/arm64/include/asm/acpi.h:12:
> > >>>      include/linux/efi.h:1093:34: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
> > >>>              status = get_var(L"SecureBoot", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size,
> > >>>                                              ^
> > >>>      include/linux/efi.h:1101:24: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
> > >>>              get_var(L"SetupMode", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size, &setupmode);
> > >>>                                    ^
> > >>>>> drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255 [-Wconstant-conversion]
> > >>>              { SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
> > >>>              ~                      ^~~~~~~~~~~~
> > >>
> > >> This is a false positive:
> > >>
> > >>>      include/linux/surface_aggregator/device.h:71:23: note: expanded from macro 'SSAM_ANY_IID'
> > >>>      #define SSAM_ANY_IID            0xffff
> > >>>                                      ^~~~~~
> > >>
> > >> Here, clang complains that the input is SSAM_ANY_ID. That is a special
> > >> value which has special considerations below. The SSAM_DEVICE() and
> > >> thus SSAM_VDEV() macros are intended to only allow either __u8 or
> > >> SSAM_ANY_ID as input in this place.
> > >>
> > >>>      include/linux/surface_aggregator/device.h:126:63: note: expanded from macro 'SSAM_VDEV'
> > >>>              SSAM_DEVICE(SSAM_DOMAIN_VIRTUAL, SSAM_VIRTUAL_TC_##cat, tid, iid, fun)
> > >>>                                                                           ^~~
> > >>>      include/linux/surface_aggregator/device.h:102:41: note: expanded from macro 'SSAM_DEVICE'
> > >>>              .instance = ((iid) != SSAM_ANY_IID) ? (iid) : 0,                        \
> > >>>                                                     ^~~
> > >>
> > >> Here is the special handling for SSAM_ANY_ID. So there is an implicit
> > >> conversion from iid, which may be __u8, to int (i.e. the type of
> > >> SSAM_ANY_ID), but there is at no point any implicit conversion of
> > >> SSAM_ANY_ID to __u8, as clang alleges.
> > >
> > > Looks like we are getting bit by https://llvm.org/pr38789 here (also
> > > tracked at https://github.com/ClangBuiltLinux/linux/issues/92).
> >
> > Thanks for the links! Looks like this is the same issue.
> >
> > >> Is there any way to silence this warning in particular without
> > >> suppressing it (e.g. by explicit casting) when users of this macro
> > >> _actually_ input any non-u8 and non-SSAM_ANY_ID value as iid,
> > >> leading to an _actual_ implicit value-changing cast?
> > >
> > > This has been worked around once before, commit b27aca2e555f ("soc:
> > > rockchip: work around clang warning"). I am not sure of another way to
> > > silence the warning in specific instances like you described aside from
> > > just fixing clang (which is obviously the correct solution) so that we
> > > will get real warnings.
> > >
> > > Unfortunately, the patch that is linked in the LLVM PR above does not
> > > appear to fix this instance.
> >
> > So what's the way forward here? Ignore this warning for now and wait for
> > the clang team to fix it or find a workaround?
> 
> I think we should fix this on the Clang side; having your additional
> test case will help us refine the initial approach.  Not the highest
> priority for us as this was found via randconfig; if it was something
> that showed up for defconfigs for every arch that would help us
> reprioritize but at the moment we have a lot of higher priority work.
> Either way, I appreciate the effort looking into this 0day bot
> warning!

Just for the record, this is visible with a regular all{mod,yes}config
build but it is just one warning so I tend to agree that it is not the
highest priority.

Cheers,
Nathan

> >
> > The best workaround I can come up with is setting the match_flags
> > parameter directly as macro argument, which would be a bit more verbose
> > and feel slightly less clean, but would do the trick.
> >
> > Also note that any changes to those macros would have to be synchronized
> > with a couple of patches which are still out to non-pdx86 trees, so if
> > we're working around this it's best to do this as soon as possible.
> > Specifically talking to Hans here, as those patches all have a
> > dependency on stuff that's only in the pdx86 tree right now. I.e.
> > getting the workaround in before setting up a tag for those dependencies
> > would simplify things.
> >
> > Regards,
> > Max
> >
> > > Cheers,
> > > Nathan
> > >
> > >> I believe GCC does get this right and only emits a warning if a
> > >> non-u8 _and_ non-SSAM_ANY_ID value is input.
> > >>
> > >> Regards,
> > >> Max
> > >>
> > >>>      3 warnings generated.
> > >>>
> > >>>
> > >>> vim +398 drivers/platform/surface/surface_aggregator_registry.c
> > >>>
> > >>>      396
> > >>>      397    static const struct ssam_device_id ssam_base_hub_match[] = {
> > >>>    > 398            { SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
> > >>>      399            { },
> > >>>      400    };
> > >>>      401
> > >>>
> > >>> ---
> > >>> 0-DAY CI Kernel Test Service, Intel Corporation
> > >>> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> > >>>
> > >>
> >
> > --
> > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/7db15c91-db37-69a4-84b7-6f58cca2ee1b%40gmail.com.
> 
> 
> 
> -- 
> Thanks,
> ~Nick Desaulniers
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [linux-next:master 1430/3917] drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255
@ 2021-03-12  3:39           ` Nathan Chancellor
  0 siblings, 0 replies; 12+ messages in thread
From: Nathan Chancellor @ 2021-03-12  3:39 UTC (permalink / raw)
  To: kbuild-all

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

On Thu, Mar 11, 2021 at 04:18:59PM -0800, Nick Desaulniers wrote:
> On Thu, Mar 11, 2021 at 4:13 PM Maximilian Luz <luzmaximilian@gmail.com> wrote:
> >
> > On 3/11/21 7:51 PM, Nathan Chancellor wrote:
> > > On Thu, Mar 11, 2021 at 02:39:03PM +0100, Maximilian Luz wrote:
> > >> On 3/11/21 2:06 PM, kernel test robot wrote:
> > >>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> > >>> head:   98546348153dee5f8ced572fd6c4690461d20f51
> > >>> commit: 797e78564634275ed4fe6b3f586c4b96eb1d86bc [1430/3917] platform/surface: aggregator_registry: Add base device hub
> > >>> config: arm64-randconfig-r026-20210311 (attached as .config)
> > >>> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 574a9dabc63ba1e7a04c08d4bde2eacd61b44ce1)
> > >>> reproduce (this is a W=1 build):
> > >>>           wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > >>>           chmod +x ~/bin/make.cross
> > >>>           # install arm64 cross compiling tool for clang build
> > >>>           # apt-get install binutils-aarch64-linux-gnu
> > >>>           # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=797e78564634275ed4fe6b3f586c4b96eb1d86bc
> > >>>           git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> > >>>           git fetch --no-tags linux-next master
> > >>>           git checkout 797e78564634275ed4fe6b3f586c4b96eb1d86bc
> > >>>           # save the attached .config to linux build tree
> > >>>           COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
> > >>>
> > >>> If you fix the issue, kindly add following tag as appropriate
> > >>> Reported-by: kernel test robot <lkp@intel.com>
> > >>>
> > >>> All warnings (new ones prefixed by >>):
> > >>>
> > >>>      In file included from drivers/platform/surface/surface_aggregator_registry.c:12:
> > >>>      In file included from include/linux/acpi.h:35:
> > >>>      In file included from include/acpi/acpi_io.h:7:
> > >>>      In file included from arch/arm64/include/asm/acpi.h:12:
> > >>>      include/linux/efi.h:1093:34: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
> > >>>              status = get_var(L"SecureBoot", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size,
> > >>>                                              ^
> > >>>      include/linux/efi.h:1101:24: warning: passing 1-byte aligned argument to 4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer access [-Walign-mismatch]
> > >>>              get_var(L"SetupMode", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size, &setupmode);
> > >>>                                    ^
> > >>>>> drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255 [-Wconstant-conversion]
> > >>>              { SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
> > >>>              ~                      ^~~~~~~~~~~~
> > >>
> > >> This is a false positive:
> > >>
> > >>>      include/linux/surface_aggregator/device.h:71:23: note: expanded from macro 'SSAM_ANY_IID'
> > >>>      #define SSAM_ANY_IID            0xffff
> > >>>                                      ^~~~~~
> > >>
> > >> Here, clang complains that the input is SSAM_ANY_ID. That is a special
> > >> value which has special considerations below. The SSAM_DEVICE() and
> > >> thus SSAM_VDEV() macros are intended to only allow either __u8 or
> > >> SSAM_ANY_ID as input in this place.
> > >>
> > >>>      include/linux/surface_aggregator/device.h:126:63: note: expanded from macro 'SSAM_VDEV'
> > >>>              SSAM_DEVICE(SSAM_DOMAIN_VIRTUAL, SSAM_VIRTUAL_TC_##cat, tid, iid, fun)
> > >>>                                                                           ^~~
> > >>>      include/linux/surface_aggregator/device.h:102:41: note: expanded from macro 'SSAM_DEVICE'
> > >>>              .instance = ((iid) != SSAM_ANY_IID) ? (iid) : 0,                        \
> > >>>                                                     ^~~
> > >>
> > >> Here is the special handling for SSAM_ANY_ID. So there is an implicit
> > >> conversion from iid, which may be __u8, to int (i.e. the type of
> > >> SSAM_ANY_ID), but there is at no point any implicit conversion of
> > >> SSAM_ANY_ID to __u8, as clang alleges.
> > >
> > > Looks like we are getting bit by https://llvm.org/pr38789 here (also
> > > tracked at https://github.com/ClangBuiltLinux/linux/issues/92).
> >
> > Thanks for the links! Looks like this is the same issue.
> >
> > >> Is there any way to silence this warning in particular without
> > >> suppressing it (e.g. by explicit casting) when users of this macro
> > >> _actually_ input any non-u8 and non-SSAM_ANY_ID value as iid,
> > >> leading to an _actual_ implicit value-changing cast?
> > >
> > > This has been worked around once before, commit b27aca2e555f ("soc:
> > > rockchip: work around clang warning"). I am not sure of another way to
> > > silence the warning in specific instances like you described aside from
> > > just fixing clang (which is obviously the correct solution) so that we
> > > will get real warnings.
> > >
> > > Unfortunately, the patch that is linked in the LLVM PR above does not
> > > appear to fix this instance.
> >
> > So what's the way forward here? Ignore this warning for now and wait for
> > the clang team to fix it or find a workaround?
> 
> I think we should fix this on the Clang side; having your additional
> test case will help us refine the initial approach.  Not the highest
> priority for us as this was found via randconfig; if it was something
> that showed up for defconfigs for every arch that would help us
> reprioritize but at the moment we have a lot of higher priority work.
> Either way, I appreciate the effort looking into this 0day bot
> warning!

Just for the record, this is visible with a regular all{mod,yes}config
build but it is just one warning so I tend to agree that it is not the
highest priority.

Cheers,
Nathan

> >
> > The best workaround I can come up with is setting the match_flags
> > parameter directly as macro argument, which would be a bit more verbose
> > and feel slightly less clean, but would do the trick.
> >
> > Also note that any changes to those macros would have to be synchronized
> > with a couple of patches which are still out to non-pdx86 trees, so if
> > we're working around this it's best to do this as soon as possible.
> > Specifically talking to Hans here, as those patches all have a
> > dependency on stuff that's only in the pdx86 tree right now. I.e.
> > getting the workaround in before setting up a tag for those dependencies
> > would simplify things.
> >
> > Regards,
> > Max
> >
> > > Cheers,
> > > Nathan
> > >
> > >> I believe GCC does get this right and only emits a warning if a
> > >> non-u8 _and_ non-SSAM_ANY_ID value is input.
> > >>
> > >> Regards,
> > >> Max
> > >>
> > >>>      3 warnings generated.
> > >>>
> > >>>
> > >>> vim +398 drivers/platform/surface/surface_aggregator_registry.c
> > >>>
> > >>>      396
> > >>>      397    static const struct ssam_device_id ssam_base_hub_match[] = {
> > >>>    > 398            { SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) },
> > >>>      399            { },
> > >>>      400    };
> > >>>      401
> > >>>
> > >>> ---
> > >>> 0-DAY CI Kernel Test Service, Intel Corporation
> > >>> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
> > >>>
> > >>
> >
> > --
> > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe(a)googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/7db15c91-db37-69a4-84b7-6f58cca2ee1b%40gmail.com.
> 
> 
> 
> -- 
> Thanks,
> ~Nick Desaulniers
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-03-12  3:39 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-11 13:06 [linux-next:master 1430/3917] drivers/platform/surface/surface_aggregator_registry.c:398:25: warning: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255 kernel test robot
2021-03-11 13:06 ` kernel test robot
2021-03-11 13:39 ` Maximilian Luz
2021-03-11 13:39   ` Maximilian Luz
2021-03-11 18:51   ` Nathan Chancellor
2021-03-11 18:51     ` Nathan Chancellor
2021-03-12  0:13     ` Maximilian Luz
2021-03-12  0:13       ` Maximilian Luz
2021-03-12  0:18       ` Nick Desaulniers
2021-03-12  0:18         ` Nick Desaulniers
2021-03-12  3:39         ` Nathan Chancellor
2021-03-12  3:39           ` Nathan Chancellor

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.