Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Bug]: C++ One Definition Rule violation #1452

Closed
ArchangeGabriel opened this issue Jul 1, 2022 · 10 comments
Closed

[Bug]: C++ One Definition Rule violation #1452

ArchangeGabriel opened this issue Jul 1, 2022 · 10 comments
Assignees
@VickyZengg @hye5
Labels
Build Cmake, build option related Common memory, surface, ddi

Comments

@ArchangeGabriel
Copy link

Which component impacted?

Build

Is it regression? Good in old configuration?

No response

What happened?

When building with LTO enabled, they are some warning:

/build/intel-media-driver/src/media-driver-intel-media-22.4.4/media_driver/agnostic/gen8/codec/hal/codechal_encode_mpeg2_g8.cpp:30: warning: type ‘struct KernelHeader’ violates the C++ One Definition Rule [-Wodr]
   30 | struct KernelHeader
      | 
/build/intel-media-driver/src/media-driver-intel-media-22.4.4/media_driver/agnostic/gen10/codec/hal/codechal_vdenc_avc_g10.cpp:39: note: a different type is defined in another translation unit
   39 | struct KernelHeader {
      | 
/build/intel-media-driver/src/media-driver-intel-media-22.4.4/media_driver/agnostic/gen8/codec/hal/codechal_encode_mpeg2_g8.cpp:32: note: the first difference of corresponding definitions is field ‘m_kernelCount’
   32 |     uint32_t m_kernelCount;
      | 
/build/intel-media-driver/src/media-driver-intel-media-22.4.4/media_driver/agnostic/gen10/codec/hal/codechal_vdenc_avc_g10.cpp:40: note: a field of same name but different type is defined in another translation unit
   40 |     int m_kernelCount;
      | 
/build/intel-media-driver/src/media-driver-intel-media-22.4.4/media_driver/agnostic/gen8/codec/hal/codechal_encode_mpeg2_g8.cpp:30: note: type ‘uint32_t’ should match type ‘int’
   30 | struct KernelHeader
      | 
/build/intel-media-driver/src/media-driver-intel-media-22.4.4/media_driver/agnostic/common/vp/hal/vphal_render_vebox_base.h:568: warning: type ‘struct _VPHAL_VEBOX_SURFACE_STATE_CMD_PARAMS’ violates the C++ One Definition Rule [-Wodr]
  568 | typedef struct _VPHAL_VEBOX_SURFACE_STATE_CMD_PARAMS
      | 
/build/intel-media-driver/src/media-driver-intel-media-22.4.4/media_softlet/agnostic/common/vp/hal/packet/vp_vebox_common.h:32: note: a different type is defined in another translation unit
   32 | typedef struct _VPHAL_VEBOX_SURFACE_STATE_CMD_PARAMS
      | 
/build/intel-media-driver/src/media-driver-intel-media-22.4.4/media_driver/agnostic/common/vp/hal/vphal_render_vebox_base.h:570: note: the first difference of corresponding definitions is field ‘pSurfInput’
  570 |     PVPHAL_SURFACE                  pSurfInput;
      | 
/build/intel-media-driver/src/media-driver-intel-media-22.4.4/media_softlet/agnostic/common/vp/hal/packet/vp_vebox_common.h:34: note: a field of same name but different type is defined in another translation unit
   34 |     PVP_SURFACE                     pSurfInput;
      | 
/build/intel-media-driver/src/media-driver-intel-media-22.4.4/media_driver/agnostic/common/vp/hal/vphal_render_vebox_base.h:568: note: type ‘struct VPHAL_SURFACE *’ should match type ‘struct VP_SURFACE *’
  568 | typedef struct _VPHAL_VEBOX_SURFACE_STATE_CMD_PARAMS
      |

What's the usage scenario when you are seeing the problem?

Others

What impacted?

No response

Debug Information

This is on 22.4.4, it might have been present before but I did not check.

Do you want to contribute a patch to fix the issue?

No response
@Jexu Jexu assigned cqian2 and unassigned Jexu, XinfengZhang and Xiaogangli-intel Dec 2, 2022
@Jexu Jexu added Build Cmake, build option related Common memory, surface, ddi labels Dec 2, 2022
@cqian2 cqian2 assigned hye5 and unassigned cqian2 Dec 4, 2022
@VickyZengg
Copy link
Contributor

VickyZengg commented Feb 3, 2023 •
edited

Hi ArchangeGabriel, I can't reproduce the issue, can you provide more details about the issue? Thanks! Such as build environment and make command.

@mattst88
Copy link
Contributor

mattst88 commented Feb 8, 2023

I confirm the issue still exists on current master. Configure and build with:

$ CFLAGS="-O2 -march=native -flto=auto -pipe" CXXFLAGS="$CFLAGS" cmake -S . -B build -G Ninja
$ ninja -C build -v

The -flto flag is what allows the compiler to recognize the issue.

[1524/1524] : && /usr/bin/c++ -fPIC -O2 -march=native -flto=auto -pipe -Wreorder -Wsign-promo -Wnon-virtual-dtor -Wno-invalid-offsetof -fvisibility-inlines-hidden -fno-use-cxa-atexit -frtti -fexceptions -fpermissive -fcheck-new -std=c++1y -std=c++11 -O3 -DNDEBUG  -Wl,--no-as-needed -Wl,--gc-sections -z relro -z now -fPIC -fstack-protector -shared -Wl,-soname,iHD_drv_video.so -o media_driver/iHD_drv_video.so @CMakeFiles/iHD_drv_video.rsp  && :
/root/projects/intel/media-driver/media_driver/agnostic/gen8/codec/hal/codechal_encode_mpeg2_g8.cpp:30: warning: type ‘struct KernelHeader’ violates the C++ One Definition Rule [-Wodr]
   30 | struct KernelHeader
      | 
/root/projects/intel/media-driver/media_driver/agnostic/gen10/codec/hal/codechal_vdenc_avc_g10.cpp:39: note: a different type is defined in another translation unit
   39 | struct KernelHeader {
      | 
/root/projects/intel/media-driver/media_driver/agnostic/gen8/codec/hal/codechal_encode_mpeg2_g8.cpp:32: note: the first difference of corresponding definitions is field ‘m_kernelCount’
   32 |     uint32_t m_kernelCount;
      | 
/root/projects/intel/media-driver/media_driver/agnostic/gen10/codec/hal/codechal_vdenc_avc_g10.cpp:40: note: a field of same name but different type is defined in another translation unit
   40 |     int m_kernelCount;
      | 
/root/projects/intel/media-driver/media_driver/agnostic/gen8/codec/hal/codechal_encode_mpeg2_g8.cpp:30: note: type ‘uint32_t’ should match type ‘int’
   30 | struct KernelHeader
      | 
/root/projects/intel/media-driver/media_driver/agnostic/common/vp/hal/vphal_render_vebox_base.h:568: warning: type ‘struct _VPHAL_VEBOX_SURFACE_STATE_CMD_PARAMS’ violates the C++ One Definition Rule [-Wodr]
  568 | typedef struct _VPHAL_VEBOX_SURFACE_STATE_CMD_PARAMS
      | 
/root/projects/intel/media-driver/media_softlet/agnostic/common/vp/hal/packet/vp_vebox_common.h:31: note: a different type is defined in another translation unit
   31 | typedef struct _VPHAL_VEBOX_SURFACE_STATE_CMD_PARAMS
      | 
/root/projects/intel/media-driver/media_driver/agnostic/common/vp/hal/vphal_render_vebox_base.h:570: note: the first difference of corresponding definitions is field ‘pSurfInput’
  570 |     PVPHAL_SURFACE                  pSurfInput;
      | 
/root/projects/intel/media-driver/media_softlet/agnostic/common/vp/hal/packet/vp_vebox_common.h:33: note: a field of same name but different type is defined in another translation unit
   33 |     PVP_SURFACE                     pSurfInput;
      | 
/root/projects/intel/media-driver/media_driver/agnostic/common/vp/hal/vphal_render_vebox_base.h:568: note: type ‘struct VPHAL_SURFACE *’ should match type ‘struct VP_SURFACE *’
  568 | typedef struct _VPHAL_VEBOX_SURFACE_STATE_CMD_PARAMS
      |````

@VickyZengg
Copy link
Contributor

I confirm the issue still exists on current master. Configure and build with:

$ CFLAGS="-O2 -march=native -flto=auto -pipe" CXXFLAGS="$CFLAGS" cmake -S . -B build -G Ninja
$ ninja -C build -v

The -flto flag is what allows the compiler to recognize the issue.

[1524/1524] : && /usr/bin/c++ -fPIC -O2 -march=native -flto=auto -pipe -Wreorder -Wsign-promo -Wnon-virtual-dtor -Wno-invalid-offsetof -fvisibility-inlines-hidden -fno-use-cxa-atexit -frtti -fexceptions -fpermissive -fcheck-new -std=c++1y -std=c++11 -O3 -DNDEBUG  -Wl,--no-as-needed -Wl,--gc-sections -z relro -z now -fPIC -fstack-protector -shared -Wl,-soname,iHD_drv_video.so -o media_driver/iHD_drv_video.so @CMakeFiles/iHD_drv_video.rsp  && :
/root/projects/intel/media-driver/media_driver/agnostic/gen8/codec/hal/codechal_encode_mpeg2_g8.cpp:30: warning: type ‘struct KernelHeader’ violates the C++ One Definition Rule [-Wodr]
   30 | struct KernelHeader
      | 
/root/projects/intel/media-driver/media_driver/agnostic/gen10/codec/hal/codechal_vdenc_avc_g10.cpp:39: note: a different type is defined in another translation unit
   39 | struct KernelHeader {
      | 
/root/projects/intel/media-driver/media_driver/agnostic/gen8/codec/hal/codechal_encode_mpeg2_g8.cpp:32: note: the first difference of corresponding definitions is field ‘m_kernelCount’
   32 |     uint32_t m_kernelCount;
      | 
/root/projects/intel/media-driver/media_driver/agnostic/gen10/codec/hal/codechal_vdenc_avc_g10.cpp:40: note: a field of same name but different type is defined in another translation unit
   40 |     int m_kernelCount;
      | 
/root/projects/intel/media-driver/media_driver/agnostic/gen8/codec/hal/codechal_encode_mpeg2_g8.cpp:30: note: type ‘uint32_t’ should match type ‘int’
   30 | struct KernelHeader
      | 
/root/projects/intel/media-driver/media_driver/agnostic/common/vp/hal/vphal_render_vebox_base.h:568: warning: type ‘struct _VPHAL_VEBOX_SURFACE_STATE_CMD_PARAMS’ violates the C++ One Definition Rule [-Wodr]
  568 | typedef struct _VPHAL_VEBOX_SURFACE_STATE_CMD_PARAMS
      | 
/root/projects/intel/media-driver/media_softlet/agnostic/common/vp/hal/packet/vp_vebox_common.h:31: note: a different type is defined in another translation unit
   31 | typedef struct _VPHAL_VEBOX_SURFACE_STATE_CMD_PARAMS
      | 
/root/projects/intel/media-driver/media_driver/agnostic/common/vp/hal/vphal_render_vebox_base.h:570: note: the first difference of corresponding definitions is field ‘pSurfInput’
  570 |     PVPHAL_SURFACE                  pSurfInput;
      | 
/root/projects/intel/media-driver/media_softlet/agnostic/common/vp/hal/packet/vp_vebox_common.h:33: note: a field of same name but different type is defined in another translation unit
   33 |     PVP_SURFACE                     pSurfInput;
      | 
/root/projects/intel/media-driver/media_driver/agnostic/common/vp/hal/vphal_render_vebox_base.h:568: note: type ‘struct VPHAL_SURFACE *’ should match type ‘struct VP_SURFACE *’
  568 | typedef struct _VPHAL_VEBOX_SURFACE_STATE_CMD_PARAMS
      |````

May I know what OS and gcc version you are using? Can you provide more details? Thanks a lot!

@mattst88
Copy link
Contributor

Gentoo Linux and gcc-12.

Are you having trouble reproducing the issue?

@haichund1
Copy link
Contributor

Gentoo

does issue only happen on Gentoo linux? what's OS ver?

@mattst88
Copy link
Contributor

mattst88 commented Mar 2, 2023

No, it doesn't happen only on Gentoo.

Please just try to reproduce it with the instructions I already gave. It's a legitimate problem in the code. E.g. there are multiple different structures named struct KernelHeader.

@VickyZengg
Copy link
Contributor

The fix already merged. Can you help verify it? Thanks a lot!
35235c2

@mattst88
Copy link
Contributor

Thanks! Looks fixed to me.

@VickyZengg
Copy link
Contributor

VickyZengg commented Apr 20, 2023 •
edited

Thanks! Looks fixed to me.

Thank you!

@ArchangeGabriel can you help verify and close this issue? Thanks a lot!

@Sherry-Lin
Copy link
Contributor

I'm going to close this issue which has been fixed in 35235c2. Please feel free to re-open it if any new issues found. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@VickyZengg VickyZengg

@hye5 hye5

Labels
Build Cmake, build option related Common memory, surface, ddi
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
10 participants
@ArchangeGabriel @mattst88 @Jexu @XinfengZhang @Sherry-Lin @Xiaogangli-intel @VickyZengg @haichund1 @hye5 @cqian2