Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 864bcb9e authored by Chris Weir's avatar Chris Weir
Browse files

Fix hwasan error for CAN HAL 

Hardware address sanitizer is complaining about parseConfigFile. I
suspect the issue is that protobuf is taking a pointer to an ifstream
rather than a reference, unique_ptr, or shared_ptr. I _think_ this
results in some sort of attempt to access the stream after it's closed.
In order to get around this, I moved ownership of the stream up one
level so that the stream stays open for longer.

Bug: 263769296
Test: canhalconfigurator-aidl doesn't crash on seahawk_hwasan-userdebug
Change-Id: I937d501a4759f0781304c518b518beaf8c6fed68
parent f274c1fb

automotive/can/aidl/default/tools/configurator/canprototools.cpp

+21 −7
Original line number Diff line number Diff line
@@ -51,15 +51,20 @@ static std::optional<std::string> readString(std::istream& s, std::streamsize n) 
    return std::string(buff, 0, std::min(n, got));

}



std::optional<CanBusConfig> parseConfigFile(const std::string& filepath) {

    std::ifstream cfg_stream(filepath);



    // text headers that would be present in a plaintext proto config file.

/*

  parseConfigFile *used to* contain the body of parseConfigStream. However, it seems there's some

  sort of odd behavior with IstreamInputStream and/or TextFormat::Parse, which causes HW Address

  Sanitizer to flag a "tag-mismatch" in this function. Having the ifstream defined in a wrapper

  function seems to solve this problem. The exact cause of this problem is yet unknown, but probably

  lies somewhere in the protobuf implementation.

*/

static __attribute__((noinline)) std::optional<CanBusConfig> parseConfigStream(

        std::ifstream& cfg_stream) {

    static const std::array<std::string, 3> text_headers = {"buses", "#", "controller"};

    auto cfg_file_snippet = readString(cfg_stream, 10);



    if (!cfg_file_snippet.has_value()) {

        LOG(ERROR) << "Can't open " << filepath << " for reading";

        LOG(ERROR) << "Can't read config from stream (maybe failed to open file?)";

        return std::nullopt;

    }

    cfg_stream.seekg(0);
@@ -77,16 +82,25 @@ std::optional<CanBusConfig> parseConfigFile(const std::string& filepath) { 
    if (text_format) {

        google::protobuf::io::IstreamInputStream pb_stream(&cfg_stream);

        if (!google::protobuf::TextFormat::Parse(&pb_stream, &config)) {

            LOG(ERROR) << "Failed to parse (text format) " << filepath;

            LOG(ERROR) << "Parsing text format config failed";

            return std::nullopt;

        }

    } else if (!config.ParseFromIstream(&cfg_stream)) {

        LOG(ERROR) << "Failed to parse (binary format) " << filepath;

        LOG(ERROR) << "Parsing binary format config failed";

        return std::nullopt;

    }

    return config;

}



std::optional<CanBusConfig> parseConfigFile(const std::string& filepath) {

    std::ifstream cfg_stream(filepath);

    auto cfg_maybe = parseConfigStream(cfg_stream);

    if (!cfg_maybe.has_value()) {

        LOG(ERROR) << "Failed to parse " << filepath;

    }

    return cfg_maybe;

}



std::optional<BusConfig> fromPbBus(const Bus& pb_bus) {

    BusConfig bus_cfg = {};

    bus_cfg.name = pb_bus.name();