automatic argument/retval display for well-known functions #158

honggyukim · 2017-09-12T15:54:06Z

Hi @namhyung and @Taeung

I'm writing some code for item 3 and I wrote a subset of working prototype.

I would like to ask you about the interface and internals.

a new option --auto-args is added
utils/autoargs.h describes the list of arguments / retvals for well known library functions
the arguments and retvals are passed as shown in the field of uftrace info.

I would like to hear your comments.

Thanks,
Honggyu

honggyukim · 2017-09-12T15:55:42Z

Here is a example usage and output:

$ uftrace --auto-args a.out 
Hello World!
# DURATION    TID     FUNCTION
   1.296 us [86017] | __monstartup();
   1.010 us [86017] | __cxa_atexit();
            [86017] | main() {
   7.497 us [86017] |   puts("Hello World!");
   6.150 us [86017] |   fflush();
   9.430 us [86017] |   malloc(0x5f5e100) = 0x7f8465ceb010;
  12.400 us [86017] |   free(0x7f8465ceb010);
  38.921 us [86017] | } /* main */

honggyukim · 2017-09-12T15:57:50Z

I would like to find a better way to specify arguments and retvals for a long list of functions. And their types as well.

namhyung

Thanks for doing this. But I think we don't need to pass the auto arg list through the env. Both of uftrace and libmcount can have the list, so it can just check the name of function and use it for argspec.

namhyung · 2017-09-12T17:12:59Z

libmcount/mcount.h

@@ -114,7 +114,7 @@ struct mcount_shmem {
 };

 /* first 4 byte saves the actual size of the argbuf */
-#define ARGBUF_SIZE  1024
+#define ARGBUF_SIZE  16384


I don't think this is needed since it's not for passing argpsec. It is used to save actual contents of arguments and return value.

I see. I just limited the maximum length to 48 in 1bed671 because some strings are too long to record. Thanks!

honggyukim · 2017-09-13T06:28:25Z

Thanks for doing this. But I think we don't need to pass the auto arg list through the env.

Hmm.. I don't clearly understand your word. I didn't pass auto arg list through env because it may exceed the limit of environment variable length. I just keep using argspec: form to save it in info file.

Both of uftrace and libmcount can have the list, so it can just check the name of function and use it for argspec.

Sorry but I don't get your point here again. Can you please explain more in details?

honggyukim · 2017-09-13T11:42:18Z

Thanks for doing this. But I think we don't need to pass the auto arg list through the env.

Hmm.. I don't clearly understand your word. I didn't pass auto arg list through env because it may exceed the limit of environment variable length. I just keep using argspec: form to save it in info file.

Clearly saying that I passed auto arg list directly to libmcount.so by including argspec.h during record time and uftrace also includes the same argspec.h to write argspec to info file. So it doesn't pass auto arg list but just use the same info for both.

namhyung · 2017-09-13T23:23:58Z

Clearly saying that I passed auto arg list directly to libmcount.so by including argspec.h during record time and uftrace also includes the same argspec.h to write argspec to info file. So it doesn't pass auto arg list but just use the same info for both.

Oh, sorry about the misunderstanding. I can see the code not passing but adding the arg string directly.

namhyung

This is a incomplete arbitrary list of standard library functions. I'm not sure how to maintain this list - it'd be great if some conversion script can be provided as well. Also it should be considered that how this can be useful in a long term.

namhyung · 2017-09-13T23:25:55Z

cmd-record.c

@@ -352,9 +354,46 @@ static int fill_file_header(struct opts *opts, int status, struct rusage *rusage
 	if (write(fd, &hdr, sizeof(hdr)) != (int)sizeof(hdr))
 		pr_err("writing header info failed");

+	char *orig_args = opts->args;
+	char *orig_retval = opts->retval;


I prefer declaring variables before any statement like in (old) C syntax.

namhyung · 2017-09-13T23:27:33Z

cmd-record.c

+
+		if (orig_args) {
+			alloc_size = strlen(auto_args_list) + strlen(orig_args) + 2;
+			opts->args = (char*)malloc(alloc_size);


Please use xmalloc (and its friends) and you don't need to cast it in C.

namhyung · 2017-09-13T23:31:38Z

cmd-record.c

+			opts->args = (char*)malloc(alloc_size);
+			strncpy(opts->args, auto_args_list, alloc_size);
+		}
+		pr_dbg("opts->args = %s\n", opts->args);


For the debug message, I think a level 1 message (pr_dbg) should be used to describe high level actions. I don't want to too many messages when using -v alone.

In this case, it may be like "auto arg is used" or "extending args using builtin auto-arg list". Messages like above can use level 2 or 3 according to the verbosity.

Thanks. I changed it to pr_dbg2.

namhyung · 2017-09-13T23:34:00Z

libmcount/mcount.c

+		if (arg_env) {
+			alloc_size = strlen(auto_args_list) + strlen(arg_env) + 2;
+			argument_str = (char*)malloc(alloc_size);
+			sprintf(argument_str, "%s;%s", auto_args_list, arg_env);


you can use xasprintf() instead.

namhyung · 2017-09-13T23:34:45Z

libmcount/mcount.c

+		else {
+			alloc_size = strlen(auto_args_list) + 1;
+			argument_str = (char*)malloc(alloc_size);
+			strncpy(argument_str, auto_args_list, alloc_size);


And xstrdup() for this.

namhyung · 2017-09-13T23:41:48Z

utils/autoargs.h

+ * TODO: need to write a script to generate this form from prototypes
+ */
+static char *auto_args_list = "\
+malloc@arg1;free@arg1;\


C concatenates adjacent string literals into one. Also you could also make it a const array rather than a pointer variable. So it could be:

static const char auto_arg_list[] = "AAA" "BBB" "CCC" ;

I changed it as a single string and separated into many pieces of string literals.

honggyukim · 2017-09-14T02:33:37Z

@namhyung I appreciate your comments in details. I applied your suggestion and updated it as a working-in-progress status. I still need to think how to generate auto args/retvals list with a script.
I just think that I can first support the function list in ltrace as specified in the link below:
https://github.com/dkogan/ltrace/blob/master/etc/libc.so.conf

honggyukim · 2017-09-14T02:39:16Z

Besides that, I would like to remove library specifier when giving args/retval before writing autoargs.h.
i.e. removing libstdc++ in malloc@libstdc++,args1

namhyung · 2017-09-14T06:21:14Z

libmcount/record.c

@@ -267,6 +267,7 @@ static unsigned save_to_argbuf(void *argbuf, struct list_head *args_spec,
 			if (str) {
 				unsigned i;
 				char *dst = ptr + 2;
+				const unsigned str_limit = 48;


For this kind of magic number, I prefer using macro constant with capital letters. Also 48 seems to too small - I don't know what's the good size but 100 looks like reasonable for me.

I also see a crash with a long string argument during uftrace dump.

For this kind of magic number, I prefer using macro constant with capital letters.

Okay. I will make it as capital letter.

Also 48 seems to too small - I don't know what's the good size but 100 looks like reasonable for me.
I also see a crash with a long string argument during uftrace dump.

I made it as 64 before but crashed when I run uftrace dump. I'm not sure about it but /usr/bin/gcc does a lot of string comparison that exceeds my expectation.

namhyung · 2017-09-14T06:29:03Z

Besides that, I would like to remove library specifier when giving args/retval before writing autoargs.h.
i.e. removing libstdc++ in malloc@libstdc++,args1

Yep, I'm working on that now..

honggyukim · 2017-09-14T06:58:03Z

@namhyung I appreciate your comments in details. I applied your suggestion and updated it as a working-in-progress status. I still need to think how to generate auto args/retvals list with a script.
I just think that I can first support the function list in ltrace as specified in the link below:
https://github.com/dkogan/ltrace/blob/master/etc/libc.so.conf

latrace by Jiri Olsa has the same feature showing library arguments/retval. I wantted to add better example headers in latrace but found that it's GPL v3. Maybe cannot use it directly so we can just generate our own list of functions.
https://github.com/jkkm/latrace/tree/master/etc/latrace.d/headers

honggyukim · 2017-09-14T07:00:02Z

Besides that, I would like to remove library specifier when giving args/retval before writing autoargs.h.
i.e. removing libstdc++ in malloc@libstdc++,args1

Yep, I'm working on that now..

I appreciate that!

honggyukim · 2017-09-14T07:16:17Z

FYI, here is some snippet of gcc trace result:

$ uftrace --nest-libcall --auto-args /usr/bin/gcc hello.c
    ...
   0.250 us [40905] | strcmp("cpp-output", "c");
   0.237 us [40905] | strcmp("c-header", "c");
   0.220 us [40905] | strcmp("c", "c");
   0.567 us [40905] | strncmp("mtune=generic", "E|M|MM:%(trad_capable_cpp) %(cpp_options) %(cp...", 1);
   0.480 us [40905] | strncmp("march=x86-64", "E|M|MM:%(trad_capable_cpp) %(cpp_options) %(cp...", 1);
   0.494 us [40905] | strncmp("mtune=generic", "M|MM:%(trad_capable_cpp) %(cpp_options) %(cpp_...", 1);
   0.476 us [40905] | strncmp("march=x86-64", "M|MM:%(trad_capable_cpp) %(cpp_options) %(cpp_...", 1);
   0.563 us [40905] | strncmp("mtune=generic", "MM:%(trad_capable_cpp) %(cpp_options) %(cpp_de...", 2);
   0.447 us [40905] | strncmp("march=x86-64", "MM:%(trad_capable_cpp) %(cpp_options) %(cpp_de...", 2);
   0.500 us [40905] | strncmp("mtune=generic", "E:%{!M:%{!MM:          %{traditional:%eGNU C n...", 1);
   0.463 us [40905] | strncmp("march=x86-64", "E:%{!M:%{!MM:          %{traditional:%eGNU C n...", 1);
   0.517 us [40905] | malloc(481) = 0xdeb7a0;
   0.433 us [40905] | memcpy(0xdeb7a0, 0x477384, 480);
   0.480 us [40905] | strncmp("mtune=generic", "M:%{!MM:          %{traditional:%eGNU C no lon...", 1);
   0.443 us [40905] | strncmp("march=x86-64", "M:%{!MM:          %{traditional:%eGNU C no lon...", 1);
   0.303 us [40905] | malloc(475) = 0xdeb990;
   0.243 us [40905] | memcpy(0xdeb990, 0xdeb7a5, 474);
    ...

In such cases, it may consume a lot of record data so I couldn't increase the limit more than 48.

honggyukim · 2017-09-18T10:08:55Z

This is a incomplete arbitrary list of standard library functions. I'm not sure how to maintain this list - it'd be great if some conversion script can be provided as well. Also it should be considered that how this can be useful in a long term.

I've added a script that generates autoargs.h from pre-defined protytypes.h header file in de3d0ec. It's not possible to put every functions at once so we can keep adding necessary functions when needed.

The example usage is as follows:

$ cat simple_prototypes.h
void *malloc(size_t size);
void free(void* ptr);
const char* strcpy(char* dest, const char* src);
int open(const char* pathname, int flags);
int pthread_mutex_lock(pthread_mutex_t *mutex);


$ ./gen-autoargs.py simple_prototypes.h
  GEN      autoargs.h


$ cat autoargs.h
    ...
static char *auto_args_list =
        "malloc@arg1/u;"
        "malloc@libstdc++,arg1/u;"
        "free@arg1/x;"
        "free@libstdc++,arg1/x;"
        "strcpy@arg1/s,arg2/s;"
        "strcpy@libstdc++,arg1/s,arg2/s;"
        "open@arg1/s,arg2;"
        "open@libstdc++,arg1/s,arg2;"
        "pthread_mutex_lock@arg1/x;"
        "pthread_mutex_lock@libstdc++,arg1/x;"
;

static char *auto_retvals_list =
        "malloc@retval/x;"
        "malloc@libstdc++,retval/x;"
        "strcpy@retval/s;"
        "strcpy@libstdc++,retval/s;"
        "open@retval;"
        "open@libstdc++,retval;"
        "pthread_mutex_lock@retval;"
        "pthread_mutex_lock@libstdc++,retval;"
;

Please take a look at it!

honggyukim · 2017-09-18T10:53:45Z

Here is the example usage:

$ uftrace --auto-args tests/t-autoargs hello 
hello 
# DURATION    TID     FUNCTION
            [11503] | main() {
   2.044 us [11503] |   strlen("autoargs test") = 13; 
   1.583 us [11503] |   calloc(1, 14) = 0x1b28a80;
   1.193 us [11503] |   free(0x1b28a80);
   1.344 us [11503] |   strcmp("hello", "hello") = 0;
   4.056 us [11503] |   puts("hello") = 6;
  18.513 us [11503] | } /* main */

honggyukim · 2017-09-18T16:21:49Z

@namhyung It seems that this work is almost done but it always generates autoargs.h whenever I run make. If you know how to fix it, please let me know. Thanks!

honggyukim · 2017-09-19T03:34:51Z

utils/gen-autoargs.py

@@ -44,7 +44,7 @@
 type_specifier.extend(["std::string"])

 # The contents of libnames will be made to genearte library name specifier for arg spec
-libnames = ["", "libstdc++"]
+libnames = ["", "libstdc++", "libc++", "libbfd", "libLLVM"]


I know this is a bit weird to add such libraries but this commit is added for reference. If library specifier is removed, then we don't need this so we can also make the args/retval list shorter.

namhyung · 2017-09-22T07:11:19Z

Makefile

@@ -193,6 +194,9 @@ $(filter-out $(objdir)/uftrace.o,$(UFTRACE_OBJS)): $(objdir)/%.o: $(srcdir)/%.c
 $(objdir)/version.h: PHONY
 	@$(srcdir)/misc/version.sh $@ $(VERSION_GIT)

+$(objdir)/autoargs.h: PHONY
+	@$(objdir)/utils/gen-autoargs.py $(objdir)/utils/prototypes.h


I think we don't need to run gen-autoargs.py always. If we provide autoargs.h, it will run only after prototype.h was changed. Also I'd like put the script and prototype header in the "misc" directory rather than "utils".

Hi, I've just updated as you said. I removed the commit for Makefile and added autoargs.h instead. Also I moved those files to misc. Thanks.

namhyung · 2017-09-23T05:25:19Z

Unlike version.sh it doesn't need to be called everytime and deleted on make clean. As it depends on prototype.h and the script itself so the following patch should work IMHO. But I think it'd be better if it could take an argument to specify name of output file.

diff --git a/Makefile b/Makefile
index a8e65d8..455371d 100644
--- a/Makefile
+++ b/Makefile
@@ -194,8 +194,8 @@ $(filter-out $(objdir)/uftrace.o,$(UFTRACE_OBJS)): $(objdir)/%.o: $(srcdir)/%.c
 $(objdir)/version.h: PHONY
        @$(srcdir)/misc/version.sh $@ $(VERSION_GIT)
 
-$(objdir)/autoargs.h: PHONY
-       @$(objdir)/utils/gen-autoargs.py $(objdir)/utils/prototypes.h
+$(objdir)/autoargs.h: $(srcdir)/utils/gen-autoargs.py $(srcdir)/utils/prototypes.h
+       $(QUIET_GEN)$(srcdir)/utils/gen-autoargs.py $(srcdir)/utils/prototypes.h
 
 $(objdir)/uftrace: $(UFTRACE_OBJS) $(UFTRACE_ARCH_OBJS) $(objdir)/libtraceevent/libtraceevent.a
        $(QUIET_LINK)$(CC) $(UFTRACE_CFLAGS) -o $@ $(UFTRACE_OBJS) $(UFTRACE_ARCH_OBJS) $(UFTRACE_LDFLAGS)
@@ -242,8 +242,7 @@ clean:
        $(Q)$(RM) $(objdir)/*.o $(objdir)/*.op $(objdir)/*.so $(objdir)/*.a
        $(Q)$(RM) $(objdir)/utils/*.o $(objdir)/utils/*.op $(objdir)/libmcount/*.op
        $(Q)$(RM) $(objdir)/gmon.out $(srcdir)/scripts/*.pyc $(TARGETS)
-       $(Q)$(RM) $(objdir)/uftrace-*.tar.gz
-       $(Q)$(RM) $(objdir)/version.h $(objdir)/autoargs.h
+       $(Q)$(RM) $(objdir)/uftrace-*.tar.gz $(objdir)/version.h
        @$(MAKE) -sC $(srcdir)/arch/$(ARCH) clean
        @$(MAKE) -sC $(srcdir)/tests ARCH=$(ARCH) clean
        @$(MAKE) -sC $(srcdir)/doc clean
diff --git a/utils/gen-autoargs.py b/utils/gen-autoargs.py
index b15451a..41e171b 100755
--- a/utils/gen-autoargs.py
+++ b/utils/gen-autoargs.py
@@ -270,5 +270,4 @@ def main(argv):
 
 
 if __name__ == "__main__":
-  print("  GEN      " + argspec_file)
   sys.exit(main(sys.argv))

honggyukim · 2017-09-24T06:42:53Z

@namhyung Thanks but how can I go back to the previous patch series in the same branch? I changed to directly add autoargs.h the force updated the same branch.

namhyung · 2017-09-24T07:16:10Z

git help reflog?

honggyukim · 2017-09-24T08:49:05Z

Thanks! I've just updated as you said, especially modified gen-autoargs.py to accept more options. Please take a look at it.

This adds *.sw[opn], *.patch and .orig rules to .gitignore. Signed-off-by: Honggyu Kim <honggyu.kp@gmail.com>

This is a preparation for adding a new option --auto-args, which displays arguments / return values for known library functions such as libc functions. Signed-off-by: Honggyu Kim <honggyu.kp@gmail.com>

Since some of strings are too long to store inside buffer, it sometimes exceeds ARGBUF_SIZE and fails. This patch limits the maximum string size is to 48 with NULL to prevent such problems. Signed-off-by: Honggyu Kim <honggyu.kp@gmail.com>

This adds a list prototypes of well-known library functions in "prototypes.h" and its parser that generates uftrace format argspec. The usage is as follows: $ ./misc/gen-autoargs.py ./misc/prototypes.h Then it generates "autoargs.h" that can be directly included by uftrace for --auto-args option support. In the below example, "simple_prototypes.h" has only 5 functions. It can be passed to "gen-autoargs.py" and the output is as follows: $ cat simple_prototypes.h void *malloc(size_t size); void free(void* ptr); const char* strcpy(char* dest, const char* src); int open(const char* pathname, int flags); int pthread_mutex_lock(pthread_mutex_t *mutex); $ ./gen-autoargs.py simple_prototypes.h GEN autoargs.h $ cat autoargs.h ... static char *auto_args_list = "malloc@arg1/u;" "free@arg1/x;" "strcpy@arg1/s,arg2/s;" "open@arg1/s,arg2;" "pthread_mutex_lock@arg1/x;" ; static char *auto_retvals_list = "malloc@retval/x;" "strcpy@retval/s;" "open@retval;" "pthread_mutex_lock@retval;" ; Signed-off-by: Honggyu Kim <honggyu.kp@gmail.com>

autoargs.h is generated by "gen-autoargs.py" based on "prototypes.h". This patch adds the generated file and this will be updated as the list of functions are updated later on. Signed-off-by: Honggyu Kim <honggyu.kp@gmail.com>

Since gen-autoargs.py generates a very long list of args/retval list, uftrace fails to store the entire list into the current buffer. It needs a huge buffer to store the list to properly support --auto-args option. This patch expands the size 8 times bigger. Signed-off-by: Honggyu Kim <honggyu.kp@gmail.com>

"utils/prototypes.h" contains the prototypes of well-known library functions, especially libc functions. uftrace understandable argspec format is generated by "gen-autoargs.py" based on "utils/prototypes.h" file. Then the generated "autoargs.h" file is directly included by uftrace for --auto-args support. This patch adds automatic args/retval rules directly to uftrace when --auto-args is enabled. It allows uftrace to display args/retval even for distributed binaries without -pg or -finstrument-functions build. Signed-off-by: Honggyu Kim <honggyu.kp@gmail.com>

$ uftrace --auto-args tests/t-autoargs hello hello # DURATION TID FUNCTION [11503] | main() { 2.044 us [11503] | strlen("autoargs test") = 13; 1.583 us [11503] | calloc(1, 14) = 0x1b28a80; 1.193 us [11503] | free(0x1b28a80); 1.344 us [11503] | strcmp("hello", "hello") = 0; 4.056 us [11503] | puts("hello") = 6; 18.513 us [11503] | } /* main */ Signed-off-by: Honggyu Kim <honggyu.kp@gmail.com>

namhyung · 2017-11-14T23:58:12Z

Merged: aeeb2b3

namhyung requested changes Sep 12, 2017

View reviewed changes

honggyukim force-pushed the check/auto-args branch 2 times, most recently from 225220c to 60ab035 Compare September 13, 2017 08:13

namhyung reviewed Sep 13, 2017

View reviewed changes

honggyukim force-pushed the check/auto-args branch from 60ab035 to 6de2824 Compare September 14, 2017 02:31

namhyung reviewed Sep 14, 2017

View reviewed changes

honggyukim force-pushed the check/auto-args branch from 6de2824 to 0f0ed4d Compare September 14, 2017 06:51

honggyukim force-pushed the check/auto-args branch 2 times, most recently from 3351468 to 9b7014e Compare September 14, 2017 07:10

honggyukim force-pushed the check/auto-args branch from 9b7014e to a1aaba9 Compare September 18, 2017 10:03

honggyukim force-pushed the check/auto-args branch from a1aaba9 to 5111b99 Compare September 18, 2017 10:31

honggyukim force-pushed the check/auto-args branch 4 times, most recently from a076aa6 to 22639e2 Compare September 18, 2017 16:18

honggyukim force-pushed the check/auto-args branch from 22639e2 to 93cfb82 Compare September 19, 2017 02:53

honggyukim commented Sep 19, 2017

View reviewed changes

namhyung reviewed Sep 22, 2017

View reviewed changes

namhyung changed the title ~~automatic argument/retval display for known functions~~ automatic argument/retval display for well-known functions Sep 22, 2017

honggyukim force-pushed the check/auto-args branch from 93cfb82 to 8771476 Compare September 22, 2017 12:10

honggyukim force-pushed the check/auto-args branch from 8771476 to 8ad4e27 Compare September 24, 2017 08:47

namhyung mentioned this pull request Oct 11, 2017

automatic argument for well-known functions #173

Closed

honggyukim added 8 commits October 14, 2017 07:51

Add some more git ignore rules

4dbd146

This adds *.sw[opn], *.patch and .orig rules to .gitignore. Signed-off-by: Honggyu Kim <honggyu.kp@gmail.com>

uftrace: Add --auto-args option

c50b14f

This is a preparation for adding a new option --auto-args, which displays arguments / return values for known library functions such as libc functions. Signed-off-by: Honggyu Kim <honggyu.kp@gmail.com>

misc: Add autoargs.h for --auto-args support

85194be

autoargs.h is generated by "gen-autoargs.py" based on "prototypes.h". This patch adds the generated file and this will be updated as the list of functions are updated later on. Signed-off-by: Honggyu Kim <honggyu.kp@gmail.com>

honggyukim force-pushed the check/auto-args branch from 8ad4e27 to 4055bfc Compare October 14, 2017 00:28

namhyung closed this Nov 14, 2017

honggyukim deleted the check/auto-args branch May 16, 2018 14:45

honggyukim added the args label May 10, 2019

honggyukim mentioned this pull request Jul 23, 2023

uftrace 주요 개발 사항 링크들 kosslab-kr/uftrace#107

Open

automatic argument/retval display for well-known functions #158

automatic argument/retval display for well-known functions #158

Conversation

honggyukim commented Sep 12, 2017

honggyukim commented Sep 12, 2017 • edited Loading

honggyukim commented Sep 12, 2017 • edited Loading

namhyung left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

honggyukim Sep 13, 2017 • edited Loading

Choose a reason for hiding this comment

honggyukim commented Sep 13, 2017 • edited Loading

honggyukim commented Sep 13, 2017

namhyung commented Sep 13, 2017

namhyung left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

honggyukim commented Sep 14, 2017 • edited Loading

honggyukim commented Sep 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

honggyukim Sep 14, 2017 • edited Loading

Choose a reason for hiding this comment

namhyung commented Sep 14, 2017

honggyukim commented Sep 14, 2017 • edited Loading

honggyukim commented Sep 14, 2017

honggyukim commented Sep 14, 2017

honggyukim commented Sep 18, 2017 • edited Loading

honggyukim commented Sep 18, 2017

honggyukim commented Sep 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

namhyung commented Sep 23, 2017 • edited Loading

honggyukim commented Sep 24, 2017 • edited Loading

namhyung commented Sep 24, 2017

honggyukim commented Sep 24, 2017

namhyung commented Nov 14, 2017

honggyukim commented Sep 12, 2017 •

edited

Loading

honggyukim commented Sep 12, 2017 •

edited

Loading

honggyukim Sep 13, 2017 •

edited

Loading

honggyukim commented Sep 13, 2017 •

edited

Loading

honggyukim commented Sep 14, 2017 •

edited

Loading

honggyukim Sep 14, 2017 •

edited

Loading

honggyukim commented Sep 14, 2017 •

edited

Loading

honggyukim commented Sep 18, 2017 •

edited

Loading

namhyung commented Sep 23, 2017 •

edited

Loading

honggyukim commented Sep 24, 2017 •

edited

Loading