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.

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

Broken stack trace on Bazel #847

Closed
marekcirkos opened this issue Aug 9, 2022 · 6 comments
Closed

Broken stack trace on Bazel #847

marekcirkos opened this issue Aug 9, 2022 · 6 comments
Assignees

Comments

@marekcirkos
Copy link
Contributor

marekcirkos commented Aug 9, 2022

With latest (0.6.0) printing stack trace on macOS with simple app built with Bazel is broken.

Code of main.cc

#include <glog/logging.h>

void bar(int x)
{
  CHECK_EQ(x, 1);
}

void foo(int x)
{
  bar(x + 3);
}

int main(int, char *argv[])
{
  google::InitGoogleLogging(argv[0]);
  google::InstallFailureSignalHandler();
  foo(1);
}

Output of main.cc with 0.6.0

$ bazel run test_app
...[bazel stuff]...
F20220809 18:34:17.086427 329099 main.cpp:5] Check failed: x == 1 (4 vs. 1) 
*** Check failure stack trace: ***
*** Aborted at 1660066457 (unix time) try "date -d @1660066457" if you are using GNU date ***
PC: @                0x0 fLB::FLAGS_version
[1]    56616 abort      bazel run test_app

compared to expected 0.5.0 output

$ bazel run test_app
...[bazel stuff]...
F20220809 18:29:14.622588 322007 main.cpp:5] Check failed: x == 1 (4 vs. 1) 
*** Check failure stack trace: ***
    @        0x10e3fc79d  google::LogMessage::Fail()
    @        0x10e3fb04a  google::LogMessage::SendToLog()
    @        0x10e3fbdea  google::LogMessage::Flush()
    @        0x10e400c19  google::LogMessageFatal::~LogMessageFatal()
    @        0x10e3fcdb5  google::LogMessageFatal::~LogMessageFatal()
    @        0x10e3f5f2a  bar()
    @        0x10e3f5fa6  foo()
    @        0x10e3f5fda  main
[1]    56075 abort      bazel run test_app

Rest of the setup

WORKSPACE

workspace(name = "glog_test")

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

http_archive(
    name = "com_github_gflags_gflags",
    sha256 = "34af2f15cf7367513b352bdcd2493ab14ce43692d2dcd9dfc499492966c64dcf",
    strip_prefix = "gflags-2.2.2",
    urls = ["https://github.com/gflags/gflags/archive/v2.2.2.tar.gz"],
)

http_archive(
    name = "com_github_google_glog",
    sha256 = "122fb6b712808ef43fbf80f75c52a21c9760683dae470154f02bddfc61135022",
    strip_prefix = "glog-0.6.0",
    urls = ["https://github.com/google/glog/archive/v0.6.0.zip"],
)

BUILD

load("@rules_cc//cc:defs.bzl", "cc_binary")

cc_binary(
    name = "test_app",
    srcs = ["main.cpp"],
    deps = ["@com_github_google_glog//:glog"],
)

Ideas

Problem seems to be related to #767.

Simply adding "-DHAVE__UNWIND_BACKTRACE", to darwin_only_copts in glog.bzl, resolves the problem.

However, I am not sure this is correct approach.

  • Should we add it to other systems as well?
@drigz
Copy link
Member

drigz commented Aug 10, 2022

Should we revert https://github.com/google/glog/pull/708/files? Or do you think it's safe to try to just undo the change to glog.bzl and rely on the CI to ensure Android stays working?

@drigz
Copy link
Member

drigz commented Aug 10, 2022

@huangqinjin It looks like #708 broke stack unwinding on other targets, but it didn't add a test for stack unwinding on Android, so I'm not sure how to verify that it still works when we try to fix it. (short of reverting #708 and adding tests on other platforms before restoring it)

Has your project pinned glog at a version that works for you? If so maybe it's easiest/safest to revert #708 before trying to fix forward. What do you think?

EDIT: Please ignore, sorry for the noise!

@drigz
Copy link
Member

drigz commented Aug 10, 2022

@marekcirkos Could you try either adding a test for this to the glog bazel tests, or using bazel build --override_repository=com_github_google_glog=path/to/glog/clone, and doing a git bisect to identify the culprit? I don't have a macOS system to repro this unfortunately.

@marekcirkos
Copy link
Contributor Author

@drigz I did bisect and actually #767 is first "offending" PR. Will look into writing test tomorrow.

@drigz
Copy link
Member

drigz commented Aug 11, 2022

Thank you for following up! Since #846, glog.bzl is the only file than refers to HAVE_UNWIND_H, so I think that should have been replaced with HAVE__UNWIND_BACKTRACE in #767. Originally I was confused because it's only defined in wasm_copts, but wasm_copts is actually used for linux+darwin+wasm (might be nice to change this but I can't think of a good name for the "opensourcey platforms other than windows"). If you can figure out a test that would be a great addition, otherwise I think it's safe to just s/HAVE_UNWIND_H/HAVE__UNWIND_BACKTRACE/ in glog.bzl.

@marekcirkos
Copy link
Contributor Author

marekcirkos commented Aug 12, 2022

#851 should help. Also added tests, hopefully will run on all platforms

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants