lib: xrefs - general cross-references & unique IDs #6766
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
This comment has been minimized.
This comment has been minimized.
|
CI:rerun |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-13198/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Merged
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result:Continuous Integration Result:
|
|
CI has passed, I just skipped the Ubuntu 16.04 ARM8 run (it's the one building up excessive backlog due to diminishing servers) |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-13441/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
|
ci:rerun |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopo tests part 1 on Ubuntu 16.04 i386: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TP1U1604I386-14590/test Topology Tests failed for Topo tests part 1 on Ubuntu 16.04 i386: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-14590/artifact/TP1U1604I386/ErrorLog/log_topotests.txt Topo tests part 0 on Ubuntu 18.04 arm8: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO0U18ARM8-14590/test Topology Tests failed for Topo tests part 0 on Ubuntu 18.04 arm8 Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopo tests part 1 on Ubuntu 16.04 i386: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TP1U1604I386-14590/test Topology Tests failed for Topo tests part 1 on Ubuntu 16.04 i386: see full log at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-14590/artifact/TP1U1604I386/ErrorLog/log_topotests.txt Topo tests part 0 on Ubuntu 18.04 arm8: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-TOPO0U18ARM8-14590/test Topology Tests failed for Topo tests part 0 on Ubuntu 18.04 arm8 Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-14590/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-14889/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-14968/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
|
I see there are some additional warnings on this ... we should probably get those cleaned up before pushing this. |
|
@eqvinox -- if we can get the warnings cleaned up, I'll push this ... if you're ready :-) |
Makes more sense to have this as a static inline. Also I don't want to be forced to link network.o into clippy ;) Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
This adds the machinery for cross reference points (hence "xref") for things to be annotated with source code location or other metadata and/or to be uniquely identified and found at runtime or by dissecting executable files. The extraction tool to walk down an ELF file is done and working but needs some more cleanup and will be added in a separate commit. Signed-off-by: David Lamparter <equinox@diac24.net>
Our "true" libraries (i.e. not modules) don't invoke neither FRR_DAEMON_INFO nor FRR_MODULE_SETUP, hence XREF_SETUP isn't invoked either. Invoke it directly to get things working. Signed-off-by: David Lamparter <equinox@diac24.net>
Just a better way of doing what was previously the "debugargdef" macro. Signed-off-by: David Lamparter <equinox@diac24.net>
This allows extracting a list of all log messages including their ECs and autogenerated unique IDs for them. Signed-off-by: David Lamparter <equinox@diac24.net>
This allows grabbing a list of all DEFUNs and their help texts through the xref extraction mechanics. Signed-off-by: David Lamparter <equinox@diac24.net>
Combined with the DEFUN xrefs, this means we can extract the full CLI tree from a binary file. Signed-off-by: David Lamparter <equinox@diac24.net>
Signed-off-by: David Lamparter <equinox@diac24.net>
Signed-off-by: David Lamparter <equinox@diac24.net>
This is the best I can make the asm blocks in lib/xref.h look, so just mute the warning. (It shouldn't come in relevant for other code.) Signed-off-by: David Lamparter <equinox@diac24.net>
|
Sorry for the tardiness, health shit really did a number on me... Style warnings should be fixed, with minor exceptions:
@riw777 @Jafaral poke (just making sure you see this due to the delay) |
... this only takes effect after the PR is merged, since the CI uses checkpatch from master, not the PR ... so the string concatenation warnings will still be there. (I did check that the change to checkpatch works.) |
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-16867/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
|
changes look good -- thanks! |
|
This is failing to build for me with this configure: This gets us closer: index 11796bc4f..891a72f2b 100644
--- a/lib/xref.h
+++ b/lib/xref.h
@@ -139,14 +139,15 @@ extern const struct xref * const __stop_xref_array[1] DSO_LOCAL;
*/
#define XREF_SETUP() \
static const struct xref _dummy_xref = { \
- .file = __FILE__, .line = __LINE__, .func = "dummy", \
- .type = XREFT_NONE, \
+ .type = XREFT_NONE, \
+ .line = __LINE__, \
+ .file = __FILE__, \
+ .func = "dummy", \
}; \
- static const struct xref * const _dummy_xref_p \
- __attribute__((used, section("xref_array"))) \
- = &_dummy_xref; \
- static void __attribute__((used, _CONSTRUCTOR(1100))) \
- _xref_init(void) { \
+ static const struct xref *const _dummy_xref_p \
+ __attribute__((used, section("xref_array"))) = &_dummy_xref; \
+ static void __attribute__((used, _CONSTRUCTOR(1100))) _xref_init(void) \
+ { \
static struct xref_block _xref_block = { \
.start = __start_xref_array, \
.stop = __stop_xref_array, \
@@ -223,16 +224,12 @@ extern const struct xref * const __stop_xref_array[1] DSO_LOCAL;
/* initializer for a "struct xref" */
#define XREF_INIT(type_, xrefdata_, func_) \
{ \
- .type = (type_), .xrefdata = (xrefdata_), \
- .file = __FILE__, .line = __LINE__, .func = func_, \
+ .xrefdata = (xrefdata_), .type = (type_), .line = __LINE__, \
+ .file = __FILE__, .func = func_, \
} \
/* end */
/* use with XREF_INIT when outside of a function, i.e. no __func__ */
#define XREF_NO_FUNC "<global>"
-#ifdef __cplusplus
-}
-#endif
-
#endif /* _FRR_XREF_H */Am still getting build failures. @eqvinox can you fix this? |
|
I suspect it's a combination of --enable-grpc and --enable-protobuf=yes |
|
Argh. Yes, this code predates the C++ compatibility push. I'm looking at it. |
|
@mwinter-osr sidenote: do we have a build with these flags ( |
|
We talked about adding these two flags to one of the CI systems in today's tech meeting. |
|
Well... it's hitting a GCC bug... from 2009... https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41091 (It works in clang.) I'm pondering what to do, the simplest thing is to disable ID generation in C++ code. (This doesn't break anything else, like the logging code can still get file/line/function from the ...but... lemme think... I could probably slap a constructor on it & get it fully functional too ... hmm |
|
Sidenote: the GCC bug might break SystemTap trace points too, I don't think those are used anywhere in C++ code yet(?) |
|
Got a workaround, I think. Testing... |
|
fixed in #8004 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is #6328 cleaned up & polished for inclusion.
the goals are:
streamlining source code location (file/line/func) references
being able to extract lists of all references like this:
https://aurora.nox.tf/tmp/xrefs-byfile.json
https://aurora.nox.tf/tmp/xrefs.json
getting automatic unique IDs for use by the daemon itself, plans include:
First iterations of this date back to 2017 in the context of log messages only. In the meantime, this is generalized / detached from log messages and implemented as a general feature.
For log messages, the unique ID is explicitly intended to change when the log message format string changes. The reason for this is that the unique ID can be used to parse / reverse printf formatting of a log message to get the individual fields of it, which only works if you have the exact format string.
The ID currently has 49 bits of entropy, I've done the math and that should be sufficient. In most cases, even the first half (
XXXXX-) should already be unique, and the dash is intentionally there due to that expectation.The extraction script (
xrelfo) is coming up in a separate PR.