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

Add first directionality test #17

Merged
merged 5 commits into from
Jun 17, 2021
Merged

Conversation

hainest
Copy link
Collaborator

@hainest hainest commented Jun 16, 2021

This just gives us a starting point for writing tests. I think the way to go is to make each test both a separate source file and generated shared library. This will keep the number of functions we need to test small and coherent. We'll also want to execute these unit tests in the Github actions for testing.

This is needed both for testing and for general use of the corpus.
@hainest hainest changed the base branch from main to refactor/register-classes June 16, 2021 04:55
@vsoch
Copy link
Member

vsoch commented Jun 16, 2021

This is so cool! And I see what you mean now. I'm testing and I have a few questions.

  1. Does the tester have any concept of getting the present working directory of the file? When I run it, it tells me it can't find the library (logically it's not in the PWD)
# ./build/test/SmeagleTests         
[doctest] doctest version is "2.4.5"
[doctest] run with "--help" for options
===============================================================================
/code/test/source/directionality.cpp:10:
TEST CASE:  Parameter Directionality

/code/test/source/directionality.cpp:10: ERROR: test case THREW exception: There was a problem reading from 'libdirectionality.so'

So then I navigated to ./build/test so the file would be found, and:

# ./SmeagleTests 
[doctest] doctest version is "2.4.5"
[doctest] run with "--help" for options
===============================================================================
/code/test/source/directionality.cpp:10:
TEST CASE:  Parameter Directionality

/code/test/source/directionality.cpp:16: FATAL ERROR: REQUIRE( funcs.size() == 1 ) is NOT correct!
  values: REQUIRE( 3 == 1 )

===============================================================================
[doctest] test cases: 3 | 2 passed | 1 failed | 0 skipped
[doctest] assertions: 2 | 1 passed | 1 failed |
[doctest] Status: FAILURE!

So it looks like it is finding three functions instead of 1! Did you run this locally and it was just one?

add_executable(SmeagleTests
source/main.cpp
source/smeagle.cpp
source/directionality.cpp
Copy link
Member

Choose a reason for hiding this comment

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

So if we have hundreds of tests, they would need to be individually listed here? Are you sure we can't do *.cpp (or some other means?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The cmake docs say that file(GLOB) isn't recursive, but it was finding files in subdirectories when I used it. Let's experiment with this.

target_link_libraries(SmeagleTests doctest::doctest Smeagle::Smeagle symtabAPI)
set_target_properties(SmeagleTests PROPERTIES CXX_STANDARD 17)

# ---- Test Libraries ----
add_library(directionality SHARED source/libs/directionality.cpp)
Copy link
Member

Choose a reason for hiding this comment

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

Same here - would we need to add them again here?

// SPDX-License-Identifier: (Apache-2.0 OR MIT)

#include <doctest/doctest.h>
#include "smeagle/smeagle.h"
Copy link
Member

Choose a reason for hiding this comment

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

I really love this setup! It will be really easy to write functions and matching tests.

@vsoch
Copy link
Member

vsoch commented Jun 16, 2021

Ahh here's the issue! I actually ran into something similar when I was parsing binaries with python. It looks like some of these default (maybe compiler added?) functions are being included:

# ./build/standalone/Smeagle -l ./build/test/libdirectionality.so 
library: "./build/test/libdirectionality.so"
locations: 
  - function:
      name: _Z3fooi
      parameters:
        - name: x
          type: int
          location: framebase+8
          direction: import
  - function:
      name: _init
      parameters:
  - function:
      name: _fini
      parameters:

There are actually more when you have more complicated binaries. So I think would it be correct to say that there should be 3? I'm not sure there is a good way to filter them out.

@vsoch
Copy link
Member

vsoch commented Jun 16, 2021

And we probably shouldn't output a field if it's empty (e.g., parameters). I'll open an issue.

@hainest
Copy link
Collaborator Author

hainest commented Jun 16, 2021

Does the tester have any concept of getting the present working directory of the file? When I run it, it tells me it can't find the library (logically it's not in the PWD)

That's a good question. I just ran it directly in the test directory, so I didn't see that. I'm not sure what the best solution is here. We can always manually set LD_LIBRARY_PATH before executing the tests, but I'm not sure how portable that is.

So it looks like it is finding three functions instead of 1! Did you run this locally and it was just one?

Oh, that's neat. What version of gcc are you using? I was using 10.2 and only found one function.

@vsoch
Copy link
Member

vsoch commented Jun 16, 2021

I'm using the gcc in the container, which is pretty old:

 gcc --version
gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

I think we should minimally develop so tests pass in the development environment (the container). Should we try updating gcc to 10.2? Are you familiar enough with Docker to try adding to this PR?

@vsoch
Copy link
Member

vsoch commented Jun 16, 2021

I'll take a shot at trying to update gcc in the container and re-running the tests - if it works I'll submit the dockerfile I created here.

@hainest
Copy link
Collaborator Author

hainest commented Jun 16, 2021

I think adding more compilers to the container would be good for extra testing. However, I would suggest we try to account for the extra functions so that we aren't writing our tests against a specific compiler. Maybe a place to start is to try to filter out the unwanted names from within the test. I'll give that a shot and see what I get locally using gcc-7.

@vsoch
Copy link
Member

vsoch commented Jun 16, 2021

Some other ones I ran into when I was developing the symbol solver:

% These are symbols to skip checking (profiling and other)
skip_symbol("_ITM_deregisterTMCloneTable").
skip_symbol("__gmon_start__").
skip_symbol("_ITM_registerTMCloneTable").

The way I was able to "filter" them out was to find linked system libraries, get their symbols, and then subtract. But we probably can't do that here. Your idea sounds good, and we can bring it up for discussion Friday to see if anyone has a better idea. I guess we can't just ignore symbols that start with an underscore?

@hainest
Copy link
Collaborator Author

hainest commented Jun 16, 2021

I guess we can't just ignore symbols that start with an underscore?

Yeah, that would be a good place to start. I think some MPI and Fortran libraries won't work with that, but we can work around those later.

@hainest
Copy link
Collaborator Author

hainest commented Jun 16, 2021

I just tried using gcc-7.5 on my local machine, but the tests passed. Where is the Dockerfile for the dev environment?

@vsoch
Copy link
Member

vsoch commented Jun 16, 2021

It's in the repository - the instructions to build and run are in the README here: https://github.com/buildsi/Smeagle/tree/refactor/register-classes#develop-using-a-container. Let me know if you reproduce!

These parameters are protected by returning a ref-to-const in
Corpus::getFunctions.
On some compilers, we get extraneous functions being included.
@vsoch vsoch merged commit 78bae50 into refactor/register-classes Jun 17, 2021
@vsoch vsoch deleted the directionality_test branch June 17, 2021 20:04
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

Successfully merging this pull request may close these issues.

None yet

2 participants