Skip to content

Conversation

@lzx404243
Copy link
Collaborator

As reported in #9601, running the unit test test_HttpTransact result in the "symbol not found" error on Debian when ATS is configured with the enable-hardening flag.

This PR adds a bunch of stubs to resolve these errors. Suggestions on any cleaner way to resolve this is also welcome :)

This resolves #9601.

@bneradt bneradt added the Build work related to build configuration or environment label Apr 17, 2023
@bneradt bneradt added this to the 10.0.0 milestone Apr 17, 2023
@bryancall bryancall requested a review from cmcfarlen April 17, 2023 22:10
@lzx404243
Copy link
Collaborator Author

lzx404243 commented Apr 17, 2023

The docker container I am doing the testing in is based off the ci.trafficserver.apache.org/ats/debian:11 image.

@cmcfarlen
Copy link
Contributor

It seems like this chunk from proxy/http/Makefile.am is what allows test_HttpTransact to even link in the first place:


if OS_LINUX
test_HttpTransact_LDFLAGS = $(AM_LDFLAGS)\
   -Wl,--unresolved-symbols=ignore-all
else
test_HttpTransact_LDFLAGS = $(AM_LDFLAGS)\
    -Wl,-undefined -Wl,suppress -Wl,-flat_namespace -Wl,-dead_strip
endif

If the hardening linker flags are specified then the symbols missing are needed for whatever security stuff happens before the code is executed or perhaps it's a problem because lazy binding is disabled. Either way these two linker flags seem to be at odds. There is a similar situation in the http3 directory as well, but these are the only two places that I found that use this flag.

As for what to do, I recommend we remove this ignore-all and link the appropriate stubs and/or libraries that are needed to link normally. This is a pain to do, so I could see the case for just merging this as is and accept that it will require maintenance from time to time. At the very least I would add a lot more commentary on why we need to stub these out, specifically mentioning the hardening configuration.

@maskit
Copy link
Member

maskit commented Apr 18, 2023

I'm the guy who added the ignore-all flag. I tried to link dependent objects in a normal way, but it was too painful and ended up adding the flag. I'm fine with removing the flag if we can run the tests, but we should probably make CI green first, and have the hardening flag one of the GitHub CI jobs so we won't be surprised after merging PRs.

@bneradt
Copy link
Contributor

bneradt commented Apr 18, 2023

and have the hardening flag one of the GitHub CI jobs so we won't be surprised after merging PRs.

+1. Once we do whatever fix we agree to with addressing this, I'll configure the debian PR CI to run with --enable-hardening

@cmcfarlen
Copy link
Contributor

Using the LDADD flags from the traffic_server makefile does work to link this test. Could use that, or start with that and try to prune some back.

@maskit
Copy link
Member

maskit commented Apr 19, 2023

Using the LDADD flags from the traffic_server makefile does work to link this test.

Well, linking everything would work for all tests for sure, but I don't want to lose the hints (objects we currently link and the stubs) we gathered so far.

@lzx404243 Can we remove the ignore-all flag if we have these stubs? Or does it still does some work?

@lzx404243
Copy link
Collaborator Author

@maskit After removing the ignore-all flag, there would be some compile-time errors such as the following:

/usr/bin/ld: ../../iocore/eventsystem/libinkevent.a(UnixEventProcessor.o): undefined reference to symbol 'hwloc_bitmap_snprintf'
/usr/bin/ld: /usr/lib/x86_64-linux-gnu/libhwloc.so.15: error adding symbols: DSO missing from command line

@maskit
Copy link
Member

maskit commented Apr 19, 2023

@lzx404243 Thanks for checking it.

I'd take this as it is, but leave the decision to @cmcfarlen .

@lzx404243 lzx404243 force-pushed the fix-debian-symbol-not-found branch from f9c9eb1 to da8eeb5 Compare April 24, 2023 16:05
@lzx404243
Copy link
Collaborator Author

@cmcfarlen Thanks for your input. I removed the stubs and the ignore-all flag, and now linking with the libs from traffic_server LDADD (with some pruning). Let me know if you have any further comments or suggestions.

@lzx404243 lzx404243 force-pushed the fix-debian-symbol-not-found branch from da8eeb5 to 4500b94 Compare April 24, 2023 16:20
Copy link
Contributor

@cmcfarlen cmcfarlen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks!

@bneradt bneradt merged commit 03a3233 into apache:master Apr 24, 2023
@zwoop
Copy link
Contributor

zwoop commented Apr 26, 2023

@bneradt This has merge conflicts on 9.2.x, can you please make a 9.2.x branch PR please?

cmcfarlen pushed a commit to cmcfarlen/trafficserver that referenced this pull request Jun 3, 2024
* asf/master: (40 commits)
  Change remap filter behavior to match ip_allow.yaml (apache#9631)
  Cleanup: Get rid of dead code from Cache (apache#9621)
  Replace obsolete Debug() macro with Dbg() in SocksProxy.cc. (apache#9613)
  Updates for the new go-httpbin v2.6.0 release. (apache#9633)
  Fix debian symbol not found for test_HttpTransact (apache#9617)
  add traffic_ctl to cmake (apache#9628)
  Fix Proxy Protocol outbound (apache#9632)
  DOC: Fix variable name `proxy.config.exec_thread.autoconfig.enabled`. (apache#9629)
  traffic_ctl: metric monitor. Handle SIGINT to drop collected stats. (apache#9570)
  traffic_ctl: plugin msg command, print out the response from server. (apache#9610)
  Doc: document IP allow filter for remap. (apache#9626)
  Cleanup: Rename d with vol (apache#9619)
  Ensure a reason phrase when sending an HTTP/1 response (apache#9615)
  Cmake plugins and install things (apache#9597)
  quic: Fix session cleanup assert. (apache#9622)
  Enables switching SSL certificates on QUIC with QUICHE (apache#9347)
  Use FetchSM for OCSP HTTP requests (apache#9591)
  Make a couple of the threads configs correct (apache#9604)
  Change submit_and_wait to take ink_hrtime. Fix test_AIO for io_uring. (apache#9555)
  Update build_h3_tools for mac (apache#9608)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Build work related to build configuration or environment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10.0.x: debian master builds fail make check due to undefined symbol: http_rsb

5 participants