Commit 3f5e1e42 authored by Denis Bessonov's avatar Denis Bessonov Committed by Commit Bot

Fix for test crash when CollectBasicGraphicsInfo fails.

The reason for change
---------------------
The tests of EXTMultisampleCompatibilityTest will crash if
CollectBasicGraphicsInfo fails. Crash occurs in the following call
graph:

TEST_F(EXTMultisampleCompatibilityTest, (any test))
  gpu::EXTMultisampleCompatibilityTest::IsApplicable()
    gpu::GPUTestBotConfig::CurrentConfigMatches("AMD")
      gpu::GPUTestBotConfig::LoadCurrentConfig(NULL)
        // Due to CollectBasicGraphicsInfo failure,
        // function makes no call SetGPUInfo() and returns true.
        // This will result in empty gpu_vendor_ vector.
        gpu::CollectBasicGraphicsInfo(&my_gpu_info)
          // Returns false for any reason
      gpu::GPUTestBotConfig::Matches(const std::string& = config_data)
        gpu::GPUTestBotConfig::Matches(const GPUConfig& = config)
          // BANG! Segfault trying to access gpu_vendor()[0]

The example of situation, where CollectBasicGraphicsInfo fails, is
a test Mac with no display connected. In this case, the internal call
graph of CollectBasicGraphicsInfo will cause the failure:

  gpu::CollectBasicGraphicsInfo(&my_gpu_info)
   angle::GetSystemInfo(&system_info)
     // Will return false after calling GetActiveGPU()
     angle::{anonimous namespace}::GetActiveGPU(...)
       // GetEntryProperty() following CGDisplayIOServicePort will
       // return false.
       CGDisplayIOServicePort(kCGDirectMainDisplay)
         // Will return MACH_PORT_NULL if display is not connected.

Suggested change
----------------
Check that gpu_vendor_ is empty and bail out early in

gpu: :GPUTestBotConfig::Matches(const GPUConfig&)
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Id8f325e4155ee928f2988c50240d7517641779f8
Reviewed-on: https://chromium-review.googlesource.com/788851
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: default avatarZhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521412}
parent 3ea2b0e7
......@@ -79,8 +79,7 @@ GPUTestConfig::OS GetCurrentOS() {
} // namespace anonymous
GPUTestConfig::GPUTestConfig()
: validate_gpu_info_(true),
os_(kOsUnknown),
: os_(kOsUnknown),
gpu_device_id_(0),
build_type_(kBuildTypeUnknown),
api_(kAPIUnknown) {}
......@@ -117,8 +116,6 @@ void GPUTestConfig::set_api(int32_t api) {
}
bool GPUTestConfig::IsValid() const {
if (!validate_gpu_info_)
return true;
if (gpu_device_id_ != 0 && (gpu_vendor_.size() != 1 || gpu_vendor_[0] == 0))
return false;
return true;
......@@ -153,10 +150,6 @@ bool GPUTestConfig::OverlapsWith(const GPUTestConfig& config) const {
return true;
}
void GPUTestConfig::DisableGPUInfoValidation() {
validate_gpu_info_ = false;
}
void GPUTestConfig::ClearGPUVendor() {
gpu_vendor_.clear();
}
......@@ -170,7 +163,6 @@ void GPUTestBotConfig::AddGPUVendor(uint32_t gpu_vendor) {
}
bool GPUTestBotConfig::SetGPUInfo(const GPUInfo& gpu_info) {
DCHECK(validate_gpu_info_);
if (gpu_info.gpu.device_id == 0 || gpu_info.gpu.vendor_id == 0)
return false;
ClearGPUVendor();
......@@ -202,12 +194,10 @@ bool GPUTestBotConfig::IsValid() const {
default:
return false;
}
if (validate_gpu_info_) {
if (gpu_vendor().size() != 1 || gpu_vendor()[0] == 0)
return false;
if (gpu_device_id() == 0)
return false;
}
switch (build_type()) {
case kBuildTypeRelease:
case kBuildTypeDebug:
......@@ -261,8 +251,7 @@ bool GPUTestBotConfig::LoadCurrentConfig(const GPUInfo* gpu_info) {
CollectInfoResult result = CollectBasicGraphicsInfo(&my_gpu_info);
if (result != kCollectInfoSuccess) {
LOG(ERROR) << "Fail to identify GPU";
DisableGPUInfoValidation();
rt = true;
rt = false;
} else {
rt = SetGPUInfo(my_gpu_info);
}
......
......@@ -83,16 +83,9 @@ class GPU_EXPORT GPUTestConfig {
// both configs.
bool OverlapsWith(const GPUTestConfig& config) const;
// Disable validation of GPU vendor and device ids.
void DisableGPUInfoValidation();
protected:
void ClearGPUVendor();
// Indicates that the OS has the notion of a numeric GPU vendor and device id
// and this data should be validated.
bool validate_gpu_info_;
private:
// operating system.
int32_t os_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment