[C++] Fix issue with valgrind unrecognized instruction

@robambalu

This is the same issue as ARROW-9851, but I dont believe the correct fix was implemented.

The merged fix lets you control if AVX512 is used by a compile flag, but that is sub-optimal because:

a) It forces you to avoid avx512 instructions solely in order to be able to run under valgrind

b) You dont always have control of the compile flags, ie if you are pip installing pyarrow

I actually reached out to the valgrind community and they insisted that pyarrow is at fault for not checking the CPU feature flags. Apparently valgrind will instrument the CPU feature flags when you run under valgrind, and it would should AVX512 is not supported, and so pyarrow should avoid using it ( I assume arrow would hit a similar issue if run on an actual CPU without AVX512 support ).

Please look into dynamically checking the CPU flags to avoid this issue and it has become a major issue for us, we cant valgrind our processes anymore now that we import arrow / pyarrow in many places.

valgrind error:

This is the error from valgrind:


vex amd64->IR: unhandled instruction bytes: 0x62 0xF1 0xFD 0x8 0x6F 0x44 0x24 0xA 0xC7 0x43
vex amd64->IR:   REX=0 REX.W=0 REX.R=0 REX.X=0 REX.B=0
vex amd64->IR:   VEX=0 VEX.L=0 VEX.nVVVV=0x0 ESC=NONE
vex amd64->IR:   PFX.66=0 PFX.F2=0 PFX.F3=0
==32035== valgrind: Unrecognised instruction at address 0x1085ea85.
==32035==    at 0x1085EA85: arrow::compute::internal::RegisterScalarAggregateSumAvx512(arrow::compute::FunctionRegistry*) (in /home/ra7293/user/hfalgo_csp/.venvs/PY36/build_tools_venv/lib/libarrow.so.100.1.0)
==32035==    by 0x1067967C: arrow::compute::GetFunctionRegistry() (in /home/ra7293/user/hfalgo_csp/.venvs/PY36/build_tools_venv/lib/libarrow.so.100.1.0)

response from valgrind dev community:


> 0x62 0xF1 0xFD 0x8 0x6F 0x44 0x24 0xA 0xC7 0x43
 
This is  vmovdqa64 0xa0(%rsp),%xmm0  which requires CPU feature flag
AVX512VL/AVX512F which is not supported by valgrind 3.18.1.
 
In the results of the emulated CPUID instruction, valgrind tells
the running application that AVX512 is not supported.
It is a bug in pyarrow ( python ) that its initialization routine
does not check the cpu feature flags, and switch its later run-time
instructions appropriately.  Please report this bug to the maintainers
of pyarrow.  Modern run-time libraries use the STT_GNU_IFUNC feature
to choose at run time which instruction set is available and may be used.
pyarrow (and/or python3 itself) must upgrade.
 
Gcc supports compile-time feature flags to avoid generating code
that uses certain instructions.  Run "info gcc" and search for "avx512",
paying attention to the flags beginning with "-mavx512", and in particular
the "-mno-avx512" variants.
 
Some software administration systems for installing and maintaining
software packages can limit the hardware features that packages
may assume.  You may be able to work-around valgrind's non-support
for AVX512 by telling the packaging system to avoid the package
variants which require AVX512.


*no* further _formatting_ is done here

Reporter: Rob Ambalu / @robambalu

_{Note: This issue was originally created as ARROW-14838. Please see the migration documentation for further details.}

@pitrou

Antoine Pitrou / @pitrou:
We already check for CPU flags before running AVX512 code, so I'm not sure what more we're supposed to do?

@robambalu

Rob Ambalu / @robambalu:
there must be something in that call stack above that is invoking the instruction?

why else would valgrind barf on it

@pitrou

Antoine Pitrou / @pitrou:
Also, I can't find RegisterScalarAggregateSumAvx512 here (I see AddSumAvx512AggKernels instead). Which version of Arrow or PyArrow are you using?

@robambalu

Rob Ambalu / @robambalu:
yes youre right, looks like it doesnt exist in the latest code. I'm currently on 1.0.1

@pitrou

Antoine Pitrou / @pitrou:
That's definitely old :-) Can you try with 6.0.1 or at least 5.0.0?

@westonpace

Weston Pace / @westonpace:
If I understand the response from Valgrind correctly the statement "Apparently valgrind will instrument the CPU feature flags when you run under valgrind, and it would should AVX512 is not supported, and so pyarrow should avoid using it" is not quite correct.

The CPU supports vmovdqa64 / AVX512. Valgrind does not support this. Arrow does not check which flags Valgrind supports when compiling (presumably most users are not going to use Valgrind and would rather have the better performance).

So at the moment I think it's pretty acceptable to say "If a developer is going to use Valgrind then the burden is on the developer to ensure they manually specify Arrow does not use AVX512".

Perhaps this is something we could add to our C++ development docs. We could maybe raise a JIRA for a new CMake option "valgrind compatibility" which disables AVX512. There is already a (perhaps hidden / not-documented) flag ARROW_TEST_MEMCHECK which disables a number of tests that are not "valgrind friendly" but I think that is an orthogonal issue.

@robambalu

Rob Ambalu / @robambalu:
Coming back tot his issue, it seems its still not resolved ( although I can work around it now by setting the SIMD env var ).
Looking at the code now, I believe there is a bug when checking CPU flags. The #if/#elif/#ifndef checks seem off, and on linux the flags are never getting set:

arrow/cpp/src/arrow/util/cpu_info.cc

Line 439 in d056829

RetrieveCPUInfo(&hardware_flags_, &model_name_, &vendor_);

This line should supposedly check the flags on non-ARM CPUs, but its under an #if _WIN32 block.
I don’t see any code path that checks CPU flags on linux.

@pitrou

Antoine Pitrou / @pitrou:
The path that checks CPU flags on Linux is here:

arrow/cpp/src/arrow/util/cpu_info.cc

Line 386 in d056829

// Read from /proc/cpuinfo

Granted, that file is a mess :-(

@robambalu

Rob Ambalu / @robambalu:
I see, but that path isn’t using cupid calls, which is what it should be using ( and what valgrind instruments )

@pitrou

Antoine Pitrou / @pitrou:
Indeed, using CPUID there would certainly be better than parsing a file. Would you like to try submitting a PR?

@pitrou

Just rechecked... the code is still identical:

arrow/cpp/src/arrow/util/cpu_info.cc

Lines 394 to 419 in 37c87db

    
           // Read from /proc/cpuinfo 
        
           // TODO: vendor, model_name for Arm 
        
           void OsRetrieveCpuInfo(int64_t* hardware_flags, CpuInfo::Vendor* vendor, 
        
                                  std::string* model_name) { 
        
             std::ifstream cpuinfo("/proc/cpuinfo", std::ios::in); 
        
             while (cpuinfo) { 
        
               std::string line; 
        
               std::getline(cpuinfo, line); 
        
               const size_t colon = line.find(':'); 
        
               if (colon != std::string::npos) { 
        
                 const std::string name = TrimString(line.substr(0, colon - 1)); 
        
                 const std::string value = TrimString(line.substr(colon + 1, std::string::npos)); 
        
                 if (name.compare("flags") == 0 || name.compare("Features") == 0) { 
        
                   *hardware_flags |= LinuxParseCpuFlags(value); 
        
                 } else if (name.compare("model name") == 0) { 
        
                   *model_name = value; 
        
                 } else if (name.compare("vendor_id") == 0) { 
        
                   if (value.compare("GenuineIntel") == 0) { 
        
                     *vendor = CpuInfo::Vendor::Intel; 
        
                   } else if (value.compare("AuthenticAMD") == 0) { 
        
                     *vendor = CpuInfo::Vendor::AMD; 
        
                   } 
        
                 } 
        
               } 
        
             } 
        
           }

reads /proc/cpuinfo, which gets the general information about what the processor is capable of (per CPU, possible bios settings, kernel boot options, possible virtualization options), which is then parsed at

arrow/cpp/src/arrow/util/cpu_info.cc

Lines 343 to 383 in 37c87db

    
           // Helper function to parse for hardware flags from /proc/cpuinfo 
        
           // values contains a list of space-separated flags.  check to see if the flags we 
        
           // care about are present. 
        
           // Returns a bitmap of flags. 
        
           int64_t LinuxParseCpuFlags(const std::string& values) { 
        
           #  if defined(CPUINFO_ARCH_X86) || defined(CPUINFO_ARCH_ARM) 
        
             const struct { 
        
               std::string name; 
        
               int64_t flag; 
        
             } flag_mappings[] = { 
        
           #    if defined(CPUINFO_ARCH_X86) 
        
               {"ssse3", CpuInfo::SSSE3}, 
        
               {"sse4_1", CpuInfo::SSE4_1}, 
        
               {"sse4_2", CpuInfo::SSE4_2}, 
        
               {"popcnt", CpuInfo::POPCNT}, 
        
               {"avx", CpuInfo::AVX}, 
        
               {"avx2", CpuInfo::AVX2}, 
        
               {"avx512f", CpuInfo::AVX512F}, 
        
               {"avx512cd", CpuInfo::AVX512CD}, 
        
               {"avx512vl", CpuInfo::AVX512VL}, 
        
               {"avx512dq", CpuInfo::AVX512DQ}, 
        
               {"avx512bw", CpuInfo::AVX512BW}, 
        
               {"bmi1", CpuInfo::BMI1}, 
        
               {"bmi2", CpuInfo::BMI2}, 
        
           #    elif defined(CPUINFO_ARCH_ARM) 
        
               {"asimd", CpuInfo::ASIMD}, 
        
           #    endif 
        
             }; 
        
             const int64_t num_flags = sizeof(flag_mappings) / sizeof(flag_mappings[0]); 
        
             int64_t flags = 0; 
        
             for (int i = 0; i < num_flags; ++i) { 
        
               if (values.find(flag_mappings[i].name) != std::string::npos) { 
        
                 flags |= flag_mappings[i].flag; 
        
               } 
        
             } 
        
             return flags; 
        
           #  else 
        
             return 0; 
        
           #  endif 
        
           }

A program can use cpuid faulting to mask features for the current run (valgrind and RR both do this) and checking via cpuid should be faster than reading and parsing the "file" in the first place.

.... not sure if @pitrou may work on a PR, but that still seems a reasonable thing to do (I'm not even an arrow-user, but still interested in the patch to learn more about how that would be coded :-)

Yes, the CpuInfo implementation is a mess, and we would like to replace most of it with a third-party library instead of trying to patch holes in it:
#45331

@AntoinePrv

(@AntoinePrv FYI)

not sure about the "widths" that were discussed in the other issue, but using cpuid to get the active features out without any parsing should be straightforward and work portable to all cpus running Linux

GH-45331: [C++] Use xsimd for CPU feature detection (#49940)

Navigation Menu

Repository navigation

[C++] Fix issue with valgrind unrecognized instruction #30368

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	// Read from /proc/cpuinfo
	// TODO: vendor, model_name for Arm
	void OsRetrieveCpuInfo(int64_t* hardware_flags, CpuInfo::Vendor* vendor,
	std::string* model_name) {
	std::ifstream cpuinfo("/proc/cpuinfo", std::ios::in);
	while (cpuinfo) {
	std::string line;
	std::getline(cpuinfo, line);
	const size_t colon = line.find(':');
	if (colon != std::string::npos) {
	const std::string name = TrimString(line.substr(0, colon - 1));
	const std::string value = TrimString(line.substr(colon + 1, std::string::npos));
	if (name.compare("flags") == 0 \|\| name.compare("Features") == 0) {
	*hardware_flags \|= LinuxParseCpuFlags(value);
	} else if (name.compare("model name") == 0) {
	*model_name = value;
	} else if (name.compare("vendor_id") == 0) {
	if (value.compare("GenuineIntel") == 0) {
	*vendor = CpuInfo::Vendor::Intel;
	} else if (value.compare("AuthenticAMD") == 0) {
	*vendor = CpuInfo::Vendor::AMD;
	}
	}
	}
	}
	}

	// Helper function to parse for hardware flags from /proc/cpuinfo
	// values contains a list of space-separated flags. check to see if the flags we
	// care about are present.
	// Returns a bitmap of flags.
	int64_t LinuxParseCpuFlags(const std::string& values) {
	# if defined(CPUINFO_ARCH_X86) \|\| defined(CPUINFO_ARCH_ARM)
	const struct {
	std::string name;
	int64_t flag;
	} flag_mappings[] = {
	# if defined(CPUINFO_ARCH_X86)
	{"ssse3", CpuInfo::SSSE3},
	{"sse4_1", CpuInfo::SSE4_1},
	{"sse4_2", CpuInfo::SSE4_2},
	{"popcnt", CpuInfo::POPCNT},
	{"avx", CpuInfo::AVX},
	{"avx2", CpuInfo::AVX2},
	{"avx512f", CpuInfo::AVX512F},
	{"avx512cd", CpuInfo::AVX512CD},
	{"avx512vl", CpuInfo::AVX512VL},
	{"avx512dq", CpuInfo::AVX512DQ},
	{"avx512bw", CpuInfo::AVX512BW},
	{"bmi1", CpuInfo::BMI1},
	{"bmi2", CpuInfo::BMI2},
	# elif defined(CPUINFO_ARCH_ARM)
	{"asimd", CpuInfo::ASIMD},
	# endif
	};
	const int64_t num_flags = sizeof(flag_mappings) / sizeof(flag_mappings[0]);

	int64_t flags = 0;
	for (int i = 0; i < num_flags; ++i) {
	if (values.find(flag_mappings[i].name) != std::string::npos) {
	flags \|= flag_mappings[i].flag;
	}
	}
	return flags;
	# else
	return 0;
	# endif
	}

[C++] Fix issue with valgrind unrecognized instruction #30368

Description

Activity

asfimport commented on Nov 24, 2021

asfimport commented on Nov 24, 2021

asfimport commented on Nov 24, 2021

asfimport commented on Nov 24, 2021

asfimport commented on Nov 24, 2021

asfimport commented on Nov 30, 2021

asfimport commented on Mar 10, 2022

asfimport commented on Mar 10, 2022

asfimport commented on Mar 10, 2022

asfimport commented on Mar 10, 2022

GitMensch commented on Sep 23, 2025

pitrou commented on Sep 23, 2025

pitrou commented on Sep 23, 2025

GitMensch commented on Sep 23, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions